[PATCH 1/8] kernel32: Implement GetVolumePathName.

Erich E. Hoover erich.e.hoover at gmail.com
Fri Jun 19 13:06:02 CDT 2015

On Fri, Jun 19, 2015 at 11:49 AM, Nikolay Sivov <bunglehead at gmail.com> wrote:
>> +    const WCHAR fallbackpathW[] = { 'C',':','\\',0 };
> This doesn't look right, could we make it dynamic using actual system drive?

It doesn't actually use the system drive, it uses C for some failures
or the first character of the path:
or the drive of the current directory:
^^ note: this conflicts with MSDN, but my tests on the testbot
indicate that MSDN is wrong

> Same goes for tests, where you got hardcoded C:\ drive.

All of those tests actually return a hard-coded C:\ value, only when
we get to patch 5 do I add a test that actually depends on the drive
of the current directory:

> According to docs this function should follow symlinks, will it potentially
> work with this implementation? Was it tested?

The behavior depends on the type of mounting, if you mount a volume
directly at a location then it will return that location:
will become:
If it's a junction point on the same drive then it will return:
If it's a junction point to a different drive (say M:) then it will
return M: on Windows.  The current implementation will return
"C:\path\to\a\mountpoint", which is enough to appease Steam.  It's
important to note that creating junction points to different drives is
known to break a lot of applications, including MS applications.

> I think it makes sense to submit tests first, maybe combining them a little,
> to have 1 tests patch instead of 4.

I split it up this way because it helped to demonstrate the special
cases in the failure codepath (and allowed me to avoid adding a bunch
of todo_wine calls unnecessarily).  Since I've actually gone the
trouble to acquire the knowledge about each of theses cases, I felt
that it was more informative to include the test and the fix together
rather than start with a dump of all the tests.


More information about the wine-devel mailing list