Bugs involving returning strings from API functions

[Justin Santa Barbara]

There are a group of functions in the kernel that appear to be broken in that they don't follow the Windows API specification.  The general convention for these functions is as follows:

The function is called with a pointer to a buffer and the size of the buffer, and returns the number of characters copied to the buffer, excluding the null terminator, unless the buffer is too small when the total size in characters of the buffer needed (with a null terminator) is returned.  If the buffer is too small, the string is not copied at all.

Several functions that I have spotted don't implement this correctly - particularly ones in files/directory.c.  eg GetSystemDirectory.
They count the Null Terminator.  A typical symptom of this is that an installation program attempts to call LoadLibrary on "C:\\WINDOWS\\SYSTEM", because it's appended eg "OLE32.DLL" after the null character.  This is one of the problems when installing the Microsoft installer, for example.  

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

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

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

However, I'm quite new to using wine, and so would like the advice and opinions of those more experienced.  My questions:
1) Is there a design reason why a single common function for string copying is not used?  Maybe something to do with separate libraries, some of which may be replaced by native windows DLLs?  Maybe someone more knowledgeable than myself could direct me as to the best place to put the helper function implementations.
2) Would writing these string helper functions and changing (most) string-returning functions to use them be worthwhile (or indeed even welcome)?
3) Is there a move to using UNICODE internally in the kernel that means that this would be best combined with some other effort?  Certainly if this were an eventual design goal then using helper functions would make that job a little easier.

Comments?

Justin SB
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://www.winehq.org/pipermail/wine-devel/attachments/20010903/3d62807f/attachment.htm