[PATCH] kernel32: Implement GetFinalPathNameByHandle

Nikolay Sivov bunglehead at gmail.com
Thu Mar 19 14:05:52 CDT 2015


On 19.03.2015 21:42, Sebastian Lackner wrote:
>
> Ah, sure. Was looking at the other code part all the time. ^^ Will fix that.
> Any other feedback while we're just discussing this version?

Sure.

> +        return GetFinalPathNameByHandleW(file, (LPWSTR)path, charcount, flags);

I don't see why you're trying to pass A buffer as a W one here. But this 
will change probably.

> +    result = GetFinalPathNameByHandleW(file, (LPWSTR)str, charcount, flags);

Do you really need a cast?

> +    if (result)
> +    {
> +        if (result < charcount)
> +        {
> +            result = FILE_name_WtoA( str, result, path, charcount - 1 );
> +            path[result] = 0;
> +        }

> +        else result--; /* Why does Windows do this? */

In this case when 'charcount <= result' (equality applies here because 
of include/don't include NULL thing?) you're still return a W-length 
instead of A one. So as I said on first reply to Andrew's path it seems 
easier to use W call in a clean way to get proper A-length at the end.

> +    }

Some comments regarding functional part (note that I didn't try to run 
test on windows or write more tests), obvious things only:

> +    else if (file == INVALID_HANDLE_VALUE)
> +    {
> +        SetLastError( ERROR_INVALID_HANDLE );
> +        return 0;
> +    }

Won't NtQueryObject() fail in this case returning proper error for you?

> +    else if (flags & ~(FILE_NAME_OPENED | VOLUME_NAME_GUID | VOLUME_NAME_NONE | VOLUME_NAME_NT))
> +    {
> +        WARN("Invalid or unsupported flags: %x\n", flags);
> +        SetLastError( ERROR_INVALID_PARAMETER );
> +        return 0;
> +    }

Invalid and unsupported is important distinction. For invalid WARN is 
fine, for unsupported FIXME is probably better, unless you tried that 
and got a flood of those in output.

>
> Sebastian
>




More information about the wine-devel mailing list