Re: [v5 1/2] ntdll: Implement FileIdInformation class support in NtQueryInformationFile.

[Sebastian Lackner]

On 14.02.2017 19:21, Jonathan Doron wrote:
> Signed-off-by: Jonathan Doron <jond(a)wizery.com>
> ---
>  dlls/ntdll/file.c       | 29 ++++++++++++++++++++++++-
>  dlls/ntdll/tests/file.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/winternl.h      |  9 ++++++++
>  3 files changed, 93 insertions(+), 1 deletion(-)

I have just sent a series (patch 130340 - 130343) based on your work which implements
initial support for FileIdInformation. There are some tasks left, however I believe it
is easier for all of us, when the basic stuff is already upstream, and we only have to
discuss about the missing pieces. ;)
> diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c
> index a6c1098..182db56 100644
> --- a/dlls/ntdll/file.c
> +++ b/dlls/ntdll/file.c
> @@ -2377,7 +2377,7 @@ NTSTATUS WINAPI NtQueryInformationFile( HANDLE hFile, PIO_STATUS_BLOCK io,
>          0,                                             /* FileRenameInformationBypassAccessCheck */
>          0,                                             /* FileLinkInformationBypassAccessCheck */
>          0,                                             /* FileVolumeNameInformation */
> -        0,                                             /* FileIdInformation */
> +        sizeof(FILE_ID_INFORMATION),                   /* FileIdInformation */
>          0,                                             /* FileIdExtdDirectoryInformation */
>          0,                                             /* FileReplaceCompletionInformation */
>          0,                                             /* FileHardLinkFullIdInformation */
> @@ -2619,6 +2619,33 @@ NTSTATUS WINAPI NtQueryInformationFile( HANDLE hFile, PIO_STATUS_BLOCK io,
>              }
>          }
>          break;
> +    case FileIdInformation:
> +        {
> +            if (fstat(fd, &st) == -1)
> +            {
> +                io->u.Status = FILE_GetNtStatus();
> +            }
> +            else
> +            {
> +                FILE_ID_INFORMATION *info = ptr;
> +#if defined(HAVE_FSTATFS)
> +                struct statfs volstats;
> +                if (fstatfs(fd, &volstats) == -1)
> +                {
> +                    io->u.Status = FILE_GetNtStatus();
> +                    break;
> +                }
> +
> +                memcpy(&info->VolumeSerialNumber,
> +                       &volstats.f_fsid,
> +                       min(sizeof(info->VolumeSerialNumber), sizeof(volstats.f_fsid)));
> +#else
> +                memset(&info->VolumeSerialNumber, 0, sizeof(info->VolumeSerialNumber));
> +#endif

It might be fine to implement the volume serial number like that, but I believe it is
even more important that all functions return the same information. If its required for
your application, please also update kernel32 and other places where it is returned.
> +                ((LARGE_INTEGER *)&info->FileId)->QuadPart = st.st_ino;
> +            }
> +        }
> +        break;
>      default:
>          FIXME("Unsupported class (%d)\n", class);
>          io->u.Status = STATUS_NOT_IMPLEMENTED;
> diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c
> index 57d7df9..926b8f8 100644
> --- a/dlls/ntdll/tests/file.c
> +++ b/dlls/ntdll/tests/file.c
> @@ -1444,6 +1444,61 @@ static void test_file_basic_information(void)
>      CloseHandle( h );
>  }
>
> +static void test_file_id_information(void)
> +{
> +    IO_STATUS_BLOCK io;
> +    FILE_ID_INFORMATION fid1, fid2;
> +    HANDLE h = INVALID_HANDLE_VALUE;
> +    NTSTATUS res;
> +    char path[MAX_PATH], buffer[MAX_PATH];
> +
> +    GetTempPathA(MAX_PATH, path);
> +    GetTempFileNameA(path, "foo", 0, buffer);
> +
> +    h = CreateFileA(buffer, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, 0);
> +    ok(h != INVALID_HANDLE_VALUE, "CreateFile error %d\n", GetLastError());
> +
> +    /* Query the temp file unique id */
> +    memset(&fid1, 0, sizeof(fid1));

The memsets with same value hide that some bytes of FileId are uninitialized.
> +    res = pNtQueryInformationFile(h, &io, &fid1, sizeof(fid1), FileIdInformation);
> +    if (res == STATUS_NOT_IMPLEMENTED || res == STATUS_INVALID_INFO_CLASS)
> +    {
> +        CloseHandle(h);
> +        DeleteFileA(buffer);
> +        win_skip("FileIdInformation not supported\n");
> +        return;
> +    }
> +
> +    ok(res == STATUS_SUCCESS, "can't get fid1, res %x\n", res);
> +
> +    CloseHandle(h);
> +
> +    h = CreateFileA(buffer, GENERIC_WRITE, 0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, 0);
> +    ok(h != INVALID_HANDLE_VALUE, "CreateFile error %d\n", GetLastError());
> +
> +    memset(&fid2, 0, sizeof(fid2));
> +    res = pNtQueryInformationFile(h, &io, &fid2, sizeof(fid2), FileIdInformation);
> +    ok(res == STATUS_SUCCESS, "can't get fid2, res %x\n", res);
> +
> +    /* Verify we got the same unique id for the file */
> +    ok(memcmp(&fid1, &fid2, sizeof(fid1)) == 0, "file id is not unique\n");
> +
> +    CloseHandle(h);
> +
> +    h = create_temp_file(0);
> +    ok(h != NULL, "error creating new file %d\n", GetLastError());
> +
> +    memset(&fid2, 0, sizeof(fid2));
> +    res = pNtQueryInformationFile(h, &io, &fid2, sizeof(fid2), FileIdInformation);
> +    ok(res == STATUS_SUCCESS, "can't get new fid2, res %x\n", res);
> +
> +    /* Verify we got the different unique id for the new file */
> +    ok(memcmp(&fid1, &fid2, sizeof(fid1)) != 0, "file id is not unique\n");
> +

Some of those patches can probably be added again if you want. For now I have
only added a test to compare the result with GetFileInformationByHandle.
> +    CloseHandle(h);
> +    DeleteFileA(buffer);
> +}
> +
>  static void test_file_all_information(void)
>  {
>      IO_STATUS_BLOCK io;
> @@ -4262,4 +4317,5 @@ START_TEST(file)
>      test_file_disposition_information();
>      test_query_volume_information_file();
>      test_query_attribute_information_file();
> +    test_file_id_information();
>  }
> diff --git a/include/winternl.h b/include/winternl.h
> index 891b6a7..2f2ea86 100644
> --- a/include/winternl.h
> +++ b/include/winternl.h
> @@ -571,6 +571,15 @@ typedef struct _FILE_INTERNAL_INFORMATION {
>      LARGE_INTEGER IndexNumber;
>  } FILE_INTERNAL_INFORMATION, *PFILE_INTERNAL_INFORMATION;
>
> +typedef struct _FILE_ID_128 {
> +    UCHAR Identifier[16];
> +} FILE_ID_128, *PFILE_ID_128;
> +
> +typedef struct _FILE_ID_INFORMATION {
> +    ULONGLONG VolumeSerialNumber;
> +    FILE_ID_128 FileId;
> +} FILE_ID_INFORMATION, *PFILE_ID_INFORMATION;
> +
>  typedef struct _FILE_EA_INFORMATION {
>      ULONG EaSize;
>  } FILE_EA_INFORMATION, *PFILE_EA_INFORMATION;