Re: kernel32: Tiny improvement to the GetVolumePathNameW stub (try 4)

[James Hawkins]

On Mon, Aug 18, 2008 at 4:18 AM, Louis. Lenders
 wrote:
> (the return TRUE statement got lost from last patch, thanks Michael for
> noticing)
>
> Hi,  this fixes Adobe Lightroom 2.0 start up bug, mentioned in
> http://bugs.winehq.org/show_bug.cgi?id=8224#c4
>
> as long wine doesn't handle with volume mount points, it's probably safe to
> satisfy most apps.
>
> (A simple google for
> "fixme:volume:GetVolumePathNameW"
> revealed only about 3 apps , and "fixme:volume:GetVolumePathNameA" revealed
> zero hits, so for now the use of this api seems not really wide-spread, and
> no need to touch GetVolumePathNameA i guess)

+    /* As long as we don't handle volumemountpoints, and filename is
something like "X:\blabla\bla", it's probably safe to return "X:"
for
+      now to keep some applications happy */
+

Please get rid of this comment.

+    if(strlenW(filename)>=3 && filename[1] == ':' && filename[2] ==
'\' && buflen >3)
+    {
+         lstrcpynW(volumepathname,filename,4);
+         return TRUE;
+    }

You've handled *a* successful case, but what about the error case?
Also, you're copying 4 bytes of filename into volumepathname.  I don't
think you understand what lstrcpyn does.  Imagine this case:

volumepathname = "aaaaaaaa"
buflen = 8
filename = "C:\file"

After the call to lstrcpyn:

volumepathname = "C:\faaaa"

kernel32: Tiny improvement to the GetVolumePathNameW stub (try 4)

[Michael Karcher]

Am Donnerstag, den 21.08.2008, 13:26 -0500 schrieb James Hawkins:
> Also, you're copying 4 bytes of filename into volumepathname.  I don't
> think you understand what lstrcpyn does.  Imagine this case:
>
> volumepathname = "aaaaaaaa"
> buflen = 8
> filename = "C:\file"
>
> After the call to lstrcpyn:
>
> volumepathname = "C:\faaaa"

No, sorry. This is not a C language issue, like Louis Lenders flamed
you, but a Win32 API issue. Still he is right in this case, because
lstrcpyn is *not* like strncpy. lstrcpyn does, according to MSDN,
perfectly the right thing here. There is even an example on the MSDN
page, stating that
  lstrcpyn(dest,"abcdefghi",4)
puts "abc" into dest.

Regards,
  Michael Karcher

kernel32: Tiny improvement to the GetVolumePathNameW stub (try 4)

[John Klehm]

On Thu, Aug 21, 2008 at 4:35 PM, Michael Karcher
 wrote:
> page, stating that
>  lstrcpyn(dest,"abcdefghi",4)
> puts "abc" into dest.

Seems like Wine's version at least always null terminates it, so
"abc\0".  Just so anyone, who like me had to look it up, can see.

http://source.winehq.org/source/dlls/kernel32/string.c#L321

Cheers,
--John

kernel32: Tiny improvement to the GetVolumePathNameW stub (try 4)

[Michael Karcher]

Am Donnerstag, den 21.08.2008, 17:35 -0500 schrieb John Klehm:
> On Thu, Aug 21, 2008 at 4:35 PM, Michael Karcher
>  wrote:
> > page, stating that
> >  lstrcpyn(dest,"abcdefghi",4)
> > puts "abc" into dest.
>
> Seems like Wine's version at least always null terminates it, so
> "abc\0".

Sorry if my words were unclear. With "abc" I meant the C language string
consisting of the letters 'a', 'b' and 'c', which by definition also has
a terminating NUL byte. lstrcpyn does *always* zero terminate the string
if the memory is accessible. According to MSDN it catches write faults
using SEH, and returns an error code. Only in this case the output could
be unterminated.

Regards,
  Michael Karcher