Bugs involving returning strings from API functions

[Uwe Bonnes]

>>>>> "Justin" == Justin Santa Barbara <justinsb at hotmail.com> writes:

    Justin> There are a group of functions in the kernel that appear to be
    Justin> broken in that they don't follow the Windows API specification.
    Justin> The general convention for these functions is as follows: The
    Justin> function is called with a pointer to a buffer and the size of
    Justin> the buffer, and returns the number of characters copied to the
    Justin> buffer, excluding the null terminator, unless the buffer is too
    Justin> small when the total size in characters of the buffer needed
    Justin> (with a null terminator) is returned.  If the buffer is too
    Justin> small, the string is not copied at all.

    Justin> Several functions that I have spotted don't implement this
    Justin> correctly - particularly ones in files/directory.c.  eg
    Justin> GetSystemDirectory.  They count the Null Terminator.

GetSystemDirectoryA returns strlen( DIR_System.short_name )
This doesn't count the  Null Terminator to my knowledge.

    Justin>  A typical
    Justin> symptom of this is that an installation program attempts to call
    Justin> LoadLibrary on "C:\\WINDOWS\\SYSTEM", because it's appended eg
    Justin> "OLE32.DLL" after the null character.  This is one of the
    Justin> problems when installing the Microsoft installer, for example.

As said above, I don't see that problem. Maybe the implementation is not
totally conforming in that it copies characters to the available buffer even
if the whole name does not fits into the buffer, but the count seems right.

    Justin> While looking for other functions that might do the same thing,
    Justin> I came across this:
    Justin> /***********************************************************************
    Justin> * GetCurrentDirectoryW (KERNEL32.@) */ UINT WINAPI
    Justin> GetCurrentDirectoryW( UINT buflen, LPWSTR buf ) { LPSTR xpath =
    Justin> HeapAlloc( GetProcessHeap(), 0, buflen+1 ); UINT ret =
    Justin> GetCurrentDirectoryA( buflen, xpath ); if (ret < buflen) ret =
    Justin> MultiByteToWideChar( CP_ACP, 0, xpath, -1, buf, buflen ) - 1;
    Justin> HeapFree( GetProcessHeap(), 0, xpath ); return ret; }

    Justin> But HeapAlloc takes a number of bytes, whereas buflen is in
    Justin> (two-byte) characters.  ie a small buffer would appear to crash
    Justin> this code. (right?)

No, as long as GetCurrentDirectoryA behaves according to the API (wich it
seemingly does), the right value is returned. 

I think it is usefull to write small test programs, challenging an (file)
API to print out the returned strings and compile it with e.g. mingw and let
it run on Linux and Windows(perhaps in VMWare).

    Justin> While I have succesfully patched these bugs on my machine, I'm
    Justin> not sure that going through and looking for every function that
    Justin> may be broken in some similar way and changing them one-by-one
    Justin> is a good way to proceed.  These bugs all have to do with
    Justin> returning strings - it would probably be better to attempt to
    Justin> have functions that 'copy out an ANSI string' or 'copy out a
    Justin> UNICODE string' according to this API.  Without meaning to be
    Justin> critical, the problem appears to be that the same functionality
    Justin> has been written many times throughout the code, and so multiple
    Justin> and different bugs have crept in.

A lot of work has been done to get things right in wine/files/.... 
So probably a some bugs remained and some functionality is duplicated.

    Justin> However, I'm quite new to using wine, and so would like the
    Justin> advice and opinions of those more experienced.  My questions: 1)
    Justin> Is there a design reason why a single common function for string
    Justin> copying is not used?  Maybe something to do with separate
    Justin> libraries, some of which may be replaced by native windows DLLs?
    Justin> Maybe someone more knowledgeable than myself could direct me as
    Justin> to the best place to put the helper function implementations.

Wine code grows since about more then 7 years with over 150 known authors,
and until noone does a big cleanup there is no clear design. And often the
Win API has no clear design, so a lot of bells and whistles have to be
observed, probably best done by (nearly) duplivating code. Also you must
observe DLL separation and restrict yourself to common C-Lib functions. If
you need some helper functions in the files handling, it best resided in ../wine/files.

    Justin> 2) Would writing these string helper functions and changing
    Justin> (most) string-returning functions to use them be worthwhile (or
    Justin> indeed even welcome)?  

If done right and tested, it seems a good thing. However as said above, it
the functions you mention I don't see the error you observe.

    Justin> 3) Is there a move to using UNICODE
    Justin> internally in the kernel that means that this would be best
    Justin> combined with some other effort?  Certainly if this were an
    Justin> eventual design goal then using helper functions would make that
    Justin> job a little easier.

It is aggreed, that Wine should implement the WINNT way of WIN32. However
the Win31/32s/95/98/... side should not be neglected. Historically Win31 and
its decendants where normaly implemented first.

So for the wine/files/... functions, the ...W functions should be
implemented genuine and the ..A functions should chunk down to that code.


    Justin> Comments?

For more discussions on wine/files/... search the archives. There have been
some definite words from Alexandre too

    Justin> Justin SB <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0

Please no HTML in mail, and set the linebreaks.

Bye
-- 
Uwe Bonnes                bon at elektron.ikp.physik.tu-darmstadt.de

Institut fuer Kernphysik  Schlossgartenstrasse 9  64289 Darmstadt
--------- Tel. 06151 162516 -------- Fax. 06151 164321 ----------