Fix lossage due to pathname truncation in calls to MODULE_LoadLibraryExA

[Michael Beach]

On Thursday 12 September 2002 20:55, Andreas Mohr wrote:
> On Thu, Sep 12, 2002 at 08:38:09PM +1000, Michael Beach wrote:
> > Recently I've been using WINE to run a Win32 exe which needs to load
> > certain DLLs which are located in the same directory as the exe. I was
> > most perplexed when I found that this worked initially, but when I moved
> > the exe and DLLs to a different directory it stopped working ie WINE
> > would refuse to start the exe, claiming that it was unable to load the
> > required DLLs.
> >
> > After some debugging, I discovered that the critical factor was the
> > length of the pathname to the exe, when it passed a certain length it
> > stopped working. However this length was way short of the MAX_PATH value
> > of 260.
>
> Ah, cool finding !

Thanks.

>
> >  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