Re: Small fix for GetCurrentDirectoryA()

[Andreas Mohr]

Hi,

On Fri, Jun 10, 2005 at 07:26:33AM +0200, Felix Nawothnig wrote:
> This makes Yoda Stories (which is shipped with a broken wavmix32.dll) 
> run out of the box.
>
> ChangeLog:
> When emulating Win9x return ERROR_INVALID_PARAMETER if a bad buffer is 
> passed to GetCurrentDirectoryA().

Does Win9x GetCurrentDirectoryA() really use an IsBadWritePtr to protect
against invalid output buffers?
Somehow I slightly doubt it...
(IsBadWritePtr isn't the fastest thing in the world...)

Aren't you covering up a previous bug with buffer allocation or so?

Andreas Mohr

Small fix for GetCurrentDirectoryA()

[Felix Nawothnig]

Andreas Mohr wrote:
> Does Win9x GetCurrentDirectoryA() really use an IsBadWritePtr to protect
> against invalid output buffers?
> Somehow I slightly doubt it...
> (IsBadWritePtr isn't the fastest thing in the world...)

Well, no. Looking at the disassembly it does about this:

static WINE_EXCEPTION_FILTER(foo)
{
     if(GetExceptionCode() & 6)
         return EXCEPTION_EXECUTE_HANDLER;
     else
         return EXCEPTION_CONTINUE_SEARCH;
}

UINT WINAPI GetCurrentDirectoryA( UINT buflen, LPSTR buf )
{
   if(buflen > 0)
   {
     __TRY {
       buf[0] += 0;
       buf[buflen] += 0;
     } __EXCEPT(foo) {
       SetLastError(ERROR_INVALID_PARAMETER);
       return 0;
     }
   }

...
}

But does the speed-difference really matter for GetCurrentDirectoryA()?
> Aren't you  covering up a previous bug with buffer allocation or so?

No. WaveMix v1.72 tries to do:

CHAR buf[MAX_PATH];
lstrcpyA(buf + GetCurrentDirectoryA(buf, sizeof(buf)), "\WAVEMIX.INI");
/* sic, they got the argument order wrong */

and since GetCurrentDirectoryA() returns 0 on failure (on Win9x) they 
end up with "\WAVEMIX.INI"...

On WinXP it crashes just as in Wine but since we strive for 
bug-compatibility...

-flx

Small fix for GetCurrentDirectoryA()

[Felix Nawothnig]

> > (IsBadWritePtr isn't the fastest thing in the world...)

Just looked at the implementation -  it does at most 
(buflen-1)/pagesize+1 memory-references, and since buflen will be 
MAX_PATH in most cases this just costs us the calling-overhead - which 
is worth the increased readability.

-flx

Small fix for GetCurrentDirectoryA()

[Uwe Bonnes]

> > > > > "Felix" == Felix Nawothnig  writes:

>>> (IsBadWritePtr isn't the fastest thing in the world...)
    Felix> Just looked at the implementation - it does at most
    Felix> (buflen-1)/pagesize+1 memory-references, and since buflen will be
    Felix> MAX_PATH in most cases this just costs us the calling-overhead -
    Felix> which is worth the increased readability.

Write a testcase for our test suite. The tes probaly needs to retrive the
Windows Version

Small fix for GetCurrentDirectoryA()

[Alexandre Julliard]

Felix Nawothnig  writes:
> > > (IsBadWritePtr isn't the fastest thing in the world...)
>
> Just looked at the implementation -  it does at most
> (buflen-1)/pagesize+1 memory-references, and since buflen will be
> MAX_PATH in most cases this just costs us the calling-overhead - which
> is worth the increased readability.

IsBadWritePtr is pretty much always the wrong thing to use, because
it's not thread safe. The right approach is to add an exception
handler directly in the function. Though in this case you could simply
check that the buffer address is not a small integer, it will catch
all instances of that bug (which is probably quite common...) without
the overhead of an exception handler.