[PATCH v2 1/2] ntdll: Return a unique volume number from NtQueryInformationFile(FileIdInformation).
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- v2: Shift the device number by 4 bytes, since the lower 4 bytes are used by the volume serial number. dlls/ntdll/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c index 7dfb0a4800e..2aa5d8f979f 100644 --- a/dlls/ntdll/file.c +++ b/dlls/ntdll/file.c @@ -2508,7 +2508,7 @@ NTSTATUS WINAPI NtQueryInformationFile( HANDLE hFile, PIO_STATUS_BLOCK io, else { FILE_ID_INFORMATION *info = ptr; - info->VolumeSerialNumber = 0; /* FIXME */ + info->VolumeSerialNumber = (ULONGLONG)st.st_dev << 32; memset( &info->FileId, 0, sizeof(info->FileId) ); *(ULONGLONG *)&info->FileId = st.st_ino; } -- 2.25.1
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/kernel32/path.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/dlls/kernel32/path.c b/dlls/kernel32/path.c index 31652d3164b..334fc25e873 100644 --- a/dlls/kernel32/path.c +++ b/dlls/kernel32/path.c @@ -257,21 +257,12 @@ DWORD WINAPI GetShortPathNameA( LPCSTR longpath, LPSTR shortpath, DWORD shortlen static BOOL is_same_file(HANDLE h1, HANDLE h2) { - int fd1; - BOOL ret = FALSE; - if (wine_server_handle_to_fd(h1, 0, &fd1, NULL) == STATUS_SUCCESS) - { - int fd2; - if (wine_server_handle_to_fd(h2, 0, &fd2, NULL) == STATUS_SUCCESS) - { - struct stat stat1, stat2; - if (fstat(fd1, &stat1) == 0 && fstat(fd2, &stat2) == 0) - ret = (stat1.st_dev == stat2.st_dev && stat1.st_ino == stat2.st_ino); - wine_server_release_fd(h2, fd2); - } - wine_server_release_fd(h1, fd1); - } - return ret; + FILE_ID_INFORMATION id1, id2; + IO_STATUS_BLOCK io; + + return !NtQueryInformationFile( h1, &io, &id1, sizeof(id1), FileIdInformation ) + && !NtQueryInformationFile( h2, &io, &id2, sizeof(id2), FileIdInformation ) + && !memcmp( &id1, &id2, sizeof(FILE_ID_INFORMATION) ); } /************************************************************************** -- 2.25.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=66394 Your paranoid android. === debiant (32 bit Chinese:China report) === kernel32: debugger.c:305: Test failed: GetThreadContext failed: 5
Hi Zebediah, On 5/3/20 2:04 pm, Zebediah Figura wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- v2: Shift the device number by 4 bytes, since the lower 4 bytes are used by the volume serial number.
dlls/ntdll/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c index 7dfb0a4800e..2aa5d8f979f 100644 --- a/dlls/ntdll/file.c +++ b/dlls/ntdll/file.c @@ -2508,7 +2508,7 @@ NTSTATUS WINAPI NtQueryInformationFile( HANDLE hFile, PIO_STATUS_BLOCK io, else { FILE_ID_INFORMATION *info = ptr; - info->VolumeSerialNumber = 0; /* FIXME */ + info->VolumeSerialNumber = (ULONGLONG)st.st_dev << 32; memset( &info->FileId, 0, sizeof(info->FileId) ); *(ULONGLONG *)&info->FileId = st.st_ino; }
Each partition is assigned a unique Serial Number when the driver is formatted and the serial never changes. The above will return, possibly, the same serial for each drive queried (assuming the drives are on the same HDD). The staging wineboot-serial patchset might be a better solution. Regards Alistair
On 3/4/20 9:04 PM, Zebediah Figura wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- v2: Shift the device number by 4 bytes, since the lower 4 bytes are used by the volume serial number.
dlls/ntdll/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c index 7dfb0a4800e..2aa5d8f979f 100644 --- a/dlls/ntdll/file.c +++ b/dlls/ntdll/file.c @@ -2508,7 +2508,7 @@ NTSTATUS WINAPI NtQueryInformationFile( HANDLE hFile, PIO_STATUS_BLOCK io, else { FILE_ID_INFORMATION *info = ptr; - info->VolumeSerialNumber = 0; /* FIXME */ + info->VolumeSerialNumber = (ULONGLONG)st.st_dev << 32; memset( &info->FileId, 0, sizeof(info->FileId) ); *(ULONGLONG *)&info->FileId = st.st_ino; }
Based on Alistair's comments and some further research, this probably isn't a great solution. It turns out that NTFS drives actually have 64-bit serial numbers, but that's truncated to 32 bits for most APIs. More importantly, the serial number doesn't change across boots, and should be unique for different removable drives—neither of which, I understand, are guarantees for st_dev. Actually, on further examination, I don't think either of our uses of is_same_file() are necessary, so I'll try to get rid of those instead.
On Mar 5, 2020, at 10:18 AM, Zebediah Figura <zfigura(a)codeweavers.com> wrote:
On 3/4/20 9:04 PM, Zebediah Figura wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- v2: Shift the device number by 4 bytes, since the lower 4 bytes are used by the volume serial number.
dlls/ntdll/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c index 7dfb0a4800e..2aa5d8f979f 100644 --- a/dlls/ntdll/file.c +++ b/dlls/ntdll/file.c @@ -2508,7 +2508,7 @@ NTSTATUS WINAPI NtQueryInformationFile( HANDLE hFile, PIO_STATUS_BLOCK io, else { FILE_ID_INFORMATION *info = ptr; - info->VolumeSerialNumber = 0; /* FIXME */ + info->VolumeSerialNumber = (ULONGLONG)st.st_dev << 32; memset( &info->FileId, 0, sizeof(info->FileId) ); *(ULONGLONG *)&info->FileId = st.st_ino; }
Based on Alistair's comments and some further research, this probably isn't a great solution.
It turns out that NTFS drives actually have 64-bit serial numbers, but that's truncated to 32 bits for most APIs. More importantly, the serial number doesn't change across boots, and should be unique for different removable drives—neither of which, I understand, are guarantees for st_dev.
Actually, on further examination, I don't think either of our uses of is_same_file() are necessary, so I'll try to get rid of those instead.
For what it's worth, macOS can provide a UUID for certain volumes, depending (I think) on the partition table type. I would think that Linux could access that, too. -Ken
On 3/5/20 10:18 AM, Zebediah Figura wrote:
On 3/4/20 9:04 PM, Zebediah Figura wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- v2: Shift the device number by 4 bytes, since the lower 4 bytes are used by the volume serial number.
dlls/ntdll/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c index 7dfb0a4800e..2aa5d8f979f 100644 --- a/dlls/ntdll/file.c +++ b/dlls/ntdll/file.c @@ -2508,7 +2508,7 @@ NTSTATUS WINAPI NtQueryInformationFile( HANDLE hFile, PIO_STATUS_BLOCK io, else { FILE_ID_INFORMATION *info = ptr; - info->VolumeSerialNumber = 0; /* FIXME */ + info->VolumeSerialNumber = (ULONGLONG)st.st_dev << 32; memset( &info->FileId, 0, sizeof(info->FileId) ); *(ULONGLONG *)&info->FileId = st.st_ino; }
Based on Alistair's comments and some further research, this probably isn't a great solution.
It turns out that NTFS drives actually have 64-bit serial numbers, but that's truncated to 32 bits for most APIs. More importantly, the serial number doesn't change across boots, and should be unique for different removable drives—neither of which, I understand, are guarantees for st_dev.
Actually, on further examination, I don't think either of our uses of is_same_file() are necessary, so I'll try to get rid of those instead.
Meh, I was wrong. The MoveFile() call can go away, but the CopyFile() call apparently really needs to stay. I guess using the real volume serial number is the best fix here, though I'd welcome better ideas. (Note that we can't just compare the file name—I checked, and CopyFile() similarly fails for hard links.)
On 05/03/2020 19:08, Zebediah Figura wrote:
On 3/5/20 10:18 AM, Zebediah Figura wrote:
On 3/4/20 9:04 PM, Zebediah Figura wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- v2: Shift the device number by 4 bytes, since the lower 4 bytes are used by the volume serial number.
dlls/ntdll/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c index 7dfb0a4800e..2aa5d8f979f 100644 --- a/dlls/ntdll/file.c +++ b/dlls/ntdll/file.c @@ -2508,7 +2508,7 @@ NTSTATUS WINAPI NtQueryInformationFile( HANDLE hFile, PIO_STATUS_BLOCK io, else { FILE_ID_INFORMATION *info = ptr; - info->VolumeSerialNumber = 0; /* FIXME */ + info->VolumeSerialNumber = (ULONGLONG)st.st_dev << 32; memset( &info->FileId, 0, sizeof(info->FileId) ); *(ULONGLONG *)&info->FileId = st.st_ino; }
Based on Alistair's comments and some further research, this probably isn't a great solution.
It turns out that NTFS drives actually have 64-bit serial numbers, but that's truncated to 32 bits for most APIs. More importantly, the serial number doesn't change across boots, and should be unique for different removable drives—neither of which, I understand, are guarantees for st_dev.
Actually, on further examination, I don't think either of our uses of is_same_file() are necessary, so I'll try to get rid of those instead.
Meh, I was wrong. The MoveFile() call can go away, but the CopyFile() call apparently really needs to stay.
I guess using the real volume serial number is the best fix here, though I'd welcome better ideas.
(Note that we can't just compare the file name—I checked, and CopyFile() similarly fails for hard links.)
Hi Zeb, FWIW, my patch for MoveFile that adds support to rename a file with a different case also depends on is_same_file. Unfortunately I resent it many times and has never been reviewed so it's not in the tree. See: https://source.winehq.org/patches/data/177833
participants (6)
-
Alistair Leslie-Hughes -
Gabriel Ivăncescu -
Ken Thomases -
Marvin -
Zebediah Figura -
Zebediah Figura