Fix lossage due to pathname truncation in calls to MODULE_LoadLibraryExA

[David Fraser]

Michael Beach wrote:

>>> static BOOL DIR_TryModulePath( LPCWSTR name, DOS_FULL_NAME *full_name,
>>>BOOL win32 ) {
>>>-    /* FIXME: for now, GetModuleFileNameW can't return more */
>>>-    /* than OFS_MAXPATHNAME. This may change with Win32. */
>>>-    WCHAR bufferW[OFS_MAXPATHNAME];
>>>+    WCHAR bufferW[MAX_PATH];
>>>     LPWSTR p;
>>>
>>>     if (!win32)
>>>@@ -727,13 +725,13 @@
>>> 	if (!GetCurrentTask()) return FALSE;
>>> 	if (!GetModuleFileName16( GetCurrentTask(), buffer, sizeof(buffer) ))
>>>             return FALSE;
>>>-        MultiByteToWideChar(CP_ACP, 0, buffer, -1, bufferW,
>>>OFS_MAXPATHNAME); +        MultiByteToWideChar(CP_ACP, 0, buffer, -1,
>>>bufferW, MAX_PATH); } else {
>>>-	if (!GetModuleFileNameW( 0, bufferW, OFS_MAXPATHNAME ) )
>>>+	if (!GetModuleFileNameW( 0, bufferW, MAX_PATH ) )
>>>             return FALSE;
>>>     }
>>>     if (!(p = strrchrW( bufferW, '\\' ))) return FALSE;
>>>-    if (OFS_MAXPATHNAME - (++p - bufferW) <= strlenW(name)) return
>>>FALSE; +    if (MAX_PATH - (++p - bufferW) <= strlenW(name)) return
>>>FALSE; strcpyW( p, name );
>>>     return DOSFS_GetFullName( bufferW, TRUE, full_name );
>>>      
>>>
>>Argl, why does this code use the buffer size contants instead of
>>sizeof(variable) !?
>>I suggest we always specify buffer length constants only *once*,
>>namely at creation of the buffer.
>>Not doing so can be potentially very harmful if we decide to change
>>the buffer length and then manage to forget one or more length constants...
>>
>>Maybe you could even also fix that "weirdness" in our code ?
>>
>>Thanks ! :)
>>    
>>
>
>I see you subscribe to the very popular "it never hurts to ask philosophy" ;-)
>
>However I don't think saying something like sizeof(bufferW) is a clear winner 
>here, as we're not interested in the size in terms of the number of bytes (or 
>number of items of data of type char to be pedantic) of bufferW, but rather 
>the number of elements in bufferW. To get the number of elements in bufferW 
>we'd have to use sizeof(bufferW) / sizeof(WCHAR), which is a bit long winded, 
>but I suppose could be wrapped by a macro.
>
>However even if we did this then instead of suffering from the "we decide to 
>change the buffer length and then manage to forget one or more length 
>constants" problem, we would instead suffer from the "we decide to change the 
>buffer type and then manage to forget one or more length calculations" 
>problem.
>
>So I can't see a compelling reason to switch to sizeof here. Of course we 
>could switch to coding in Ada, and take advantage of attributes to tell us 
>exactly what we wanted to know about the variable, or perhaps coding in C++ 
>where we could accomplish much the same thing using some template-based smoke 
>and mirrors. No, I'm not serious ;-)
>
>Regards
>M.Beach
>  
>
What about defining the length of the buffer as a constant integer

    const int bufferW_len = MAX_PATH;
    WCHAR bufferW[bufferW_len];


Then we can use bufferW_len whenever we need to refer to the length of the array
in the function, without worrying about the size or type, and still only
change it in one place. Also it is clear in the definition what we're doing

Just an idea :-)