[PATCH v6 5/5] kernelbase: Reimplement GetVolumeInformation on top of GetVolumeInformationByHandle.

[Erich E. Hoover]

This patch removes the volume querying information duplication in
kernelbase by re-implementing GetVolumeInformation using
GetVolumeInformationByHandle (itself implemented with
NtQueryVolumeInformationFile).

v6: No change
v5: Rebase against 1b1b6c5f7d28768feb853eefd015aa714b819ed4
v4: Reordered (no content change)
v3: No change
v2: Reordered (no content change)

Best,
Erich

[Zebediah Figura (she/her)]

Just a couple small nitpicks...

On 2/6/21 12:26 PM, Erich E. Hoover wrote:
> From: "Erich E. Hoover" <erich.e.hoover(a)gmail.com>
> Subject: [PATCH v6 5/5] kernelbase: Reimplement GetVolumeInformation on top of GetVolumeInformationByHandle.
> Message-Id: <CAEU2+vrAcFN5oXJvunL0umLXW4W0je0K41cEfEtPkAkDre=StQ(a)mail.gmail.com>
> Date: Sat, 6 Feb 2021 11:26:05 -0700
>
> This patch removes the volume querying information duplication in
> kernelbase by re-implementing GetVolumeInformation using
> GetVolumeInformationByHandle (itself implemented with
> NtQueryVolumeInformationFile).
>
> v6: No change
> v5: Rebase against 1b1b6c5f7d28768feb853eefd015aa714b819ed4
> v4: Reordered (no content change)
> v3: No change
> v2: Reordered (no content change)
>
> Best,
> Erich
>
> From ca214462c58c706ada239bb11a39bb44fa7f6541 Mon Sep 17 00:00:00 2001
> From: "Erich E. Hoover" <erich.e.hoover(a)gmail.com>
> Date: Fri, 15 May 2020 22:07:10 -0600
> Subject: kernelbase: Reimplement GetVolumeInformation on top of
>  GetVolumeInformationByHandle.
>
> Signed-off-by: Erich E. Hoover <erich.e.hoover(a)gmail.com>
> ---
>  dlls/kernel32/tests/volume.c |  40 +++
>  dlls/kernelbase/volume.c     | 595 +----------------------------------
>  2 files changed, 46 insertions(+), 589 deletions(-)
>
> diff --git a/dlls/kernel32/tests/volume.c b/dlls/kernel32/tests/volume.c
> index 5199a4992d8..6f8884c7bc8 100644
> --- a/dlls/kernel32/tests/volume.c
> +++ b/dlls/kernel32/tests/volume.c
> @@ -1522,6 +1522,8 @@ static void test_GetVolumeInformationByHandle(void)
>      FILE_FS_VOLUME_INFORMATION *volume_info = (void *)buffer;
>      DWORD serial, filename_len, flags;
>      WCHAR label[20], fsname[20];
> +    char Root_Slash[]="C:\\";

Why is this a local variable?
> +    char volume[MAX_PATH+1];
>      IO_STATUS_BLOCK io;
>      HANDLE file;
>      NTSTATUS status;
> @@ -1578,6 +1580,44 @@ static void test_GetVolumeInformationByHandle(void)
>              "expected label length %u, got %u\n", volume_info->VolumeLabelLength / sizeof(WCHAR), wcslen( label ));
>
> CloseHandle( file );
> +
> +    /* get the unique volume name for the windows drive  */
> +    ret = GetVolumeNameForVolumeMountPointA(Root_Slash, volume, MAX_PATH);
> +    ok(ret == TRUE, "GetVolumeNameForVolumeMountPointA failed\n");
> +
> +    /* try again with unique volume name */
> +
> +    file = CreateFileA( volume, 0, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL,
> +            OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL );
> +    ok(file != INVALID_HANDLE_VALUE, "failed to open file, error %u\n", GetLastError());
> +
> +    ret = pGetVolumeInformationByHandleW( file, label, ARRAY_SIZE(label), &serial,
> +            &filename_len, &flags, fsname, ARRAY_SIZE(fsname) );
> +    ok(ret, "got error %u\n", GetLastError());
> +
> +    memset(buffer, 0, sizeof(buffer));
> +    status = NtQueryVolumeInformationFile( file, &io, buffer, sizeof(buffer), FileFsVolumeInformation );
> +    ok(!status, "got status %#x\n", status);
> +    ok(serial == volume_info->VolumeSerialNumber, "expected serial %08x, got %08x\n",
> +            volume_info->VolumeSerialNumber, serial);
> +    ok(!wcscmp( label, volume_info->VolumeLabel ), "expected label %s, got %s\n",
> +            debugstr_w( volume_info->VolumeLabel ), debugstr_w( label ));
> +    ok(wcslen( label ) == volume_info->VolumeLabelLength / sizeof(WCHAR),
> +            "expected label length %u, got %u\n", volume_info->VolumeLabelLength / sizeof(WCHAR), wcslen( label ));
> +
> +    memset(buffer, 0, sizeof(buffer));
> +    status = NtQueryVolumeInformationFile( file, &io, buffer, sizeof(buffer), FileFsAttributeInformation );
> +    ok(!status, "got status %#x\n", status);
> +    ok(flags == attr_info->FileSystemAttributes, "expected flags %#x, got %#x\n",
> +            attr_info->FileSystemAttributes, flags);
> +    ok(filename_len == attr_info->MaximumComponentNameLength, "expected filename_len %u, got %u\n",
> +            attr_info->MaximumComponentNameLength, filename_len);
> +    ok(!wcscmp( fsname, attr_info->FileSystemName ), "expected fsname %s, got %s\n",
> +            debugstr_w( attr_info->FileSystemName ), debugstr_w( fsname ));
> +    ok(wcslen( fsname ) == attr_info->FileSystemNameLength / sizeof(WCHAR),
> +            "expected fsname length %u, got %u\n", attr_info->FileSystemNameLength / sizeof(WCHAR), wcslen( fsname ));
> +
> +    CloseHandle( file );
>  }
>
> START_TEST(volume)
> diff --git a/dlls/kernelbase/volume.c b/dlls/kernelbase/volume.c
> index 7daf9c98823..3c4e2fd2ec1 100644
> --- a/dlls/kernelbase/volume.c
> +++ b/dlls/kernelbase/volume.c
> @@ -149,500 +149,6 @@ static DWORD get_mountmgr_drive_type( LPCWSTR root )
>      return data.type;
>  }
>
> -/* get the label by reading it from a file at the root of the filesystem */
> -static void get_filesystem_label( const UNICODE_STRING *device, WCHAR *label, DWORD len )
> -{
> -    HANDLE handle;
> -    UNICODE_STRING name;
> -    IO_STATUS_BLOCK io;
> -    OBJECT_ATTRIBUTES attr;
> -
> -    label[0] = 0;
> -
> -    attr.Length = sizeof(attr);
> -    attr.RootDirectory = 0;
> -    attr.Attributes = OBJ_CASE_INSENSITIVE;
> -    attr.ObjectName = &name;
> -    attr.SecurityDescriptor = NULL;
> -    attr.SecurityQualityOfService = NULL;
> -
> -    name.MaximumLength = device->Length + sizeof(L".windows-label");
> -    name.Length = name.MaximumLength - sizeof(WCHAR);
> -    if (!(name.Buffer = HeapAlloc( GetProcessHeap(), 0, name.MaximumLength ))) return;
> -
> -    memcpy( name.Buffer, device->Buffer, device->Length );
> -    memcpy( name.Buffer + device->Length / sizeof(WCHAR), L".windows-label", sizeof(L".windows-label") );
> -    if (!NtOpenFile( &handle, GENERIC_READ | SYNCHRONIZE, &attr, &io, FILE_SHARE_READ|FILE_SHARE_WRITE,
> -                     FILE_NON_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT ))
> -    {
> -        char buffer[256], *p;
> -        DWORD size;
> -
> -        if (!ReadFile( handle, buffer, sizeof(buffer)-1, &size, NULL )) size = 0;
> -        CloseHandle( handle );
> -        p = buffer + size;
> -        while (p > buffer && (p[-1] == ' ' || p[-1] == '\r' || p[-1] == '\n')) p--;
> -        *p = 0;
> -        if (!MultiByteToWideChar( CP_UNIXCP, 0, buffer, -1, label, len ))
> -            label[len-1] = 0;
> -    }
> -    RtlFreeUnicodeString( &name );
> -}
> -
> -/* get the serial number by reading it from a file at the root of the filesystem */
> -static DWORD get_filesystem_serial( const UNICODE_STRING *device )
> -{
> -    HANDLE handle;
> -    UNICODE_STRING name;
> -    IO_STATUS_BLOCK io;
> -    OBJECT_ATTRIBUTES attr;
> -    DWORD ret = 0;
> -
> -    attr.Length = sizeof(attr);
> -    attr.RootDirectory = 0;
> -    attr.Attributes = OBJ_CASE_INSENSITIVE;
> -    attr.ObjectName = &name;
> -    attr.SecurityDescriptor = NULL;
> -    attr.SecurityQualityOfService = NULL;
> -
> -    name.MaximumLength = device->Length + sizeof(L".windows-serial");
> -    name.Length = name.MaximumLength - sizeof(WCHAR);
> -    if (!(name.Buffer = HeapAlloc( GetProcessHeap(), 0, name.MaximumLength ))) return 0;
> -
> -    memcpy( name.Buffer, device->Buffer, device->Length );
> -    memcpy( name.Buffer + device->Length / sizeof(WCHAR), L".windows-serial", sizeof(L".windows-serial") );
> -    if (!NtOpenFile( &handle, GENERIC_READ | SYNCHRONIZE, &attr, &io, FILE_SHARE_READ|FILE_SHARE_WRITE,
> -                     FILE_SYNCHRONOUS_IO_NONALERT ))
> -    {
> -        char buffer[32];
> -        DWORD size;
> -
> -        if (!ReadFile( handle, buffer, sizeof(buffer)-1, &size, NULL )) size = 0;
> -        CloseHandle( handle );
> -        buffer[size] = 0;
> -        ret = strtoul( buffer, NULL, 16 );
> -    }
> -    RtlFreeUnicodeString( &name );
> -    return ret;
> -}
> -
> -
> -/******************************************************************
> - *		find_cdrom_best_voldesc
> - */
> -static DWORD find_cdrom_best_voldesc( HANDLE handle )
> -{
> -    BYTE cur_vd_type, max_vd_type = 0;
> -    BYTE buffer[0x800];
> -    DWORD size, offs, best_offs = 0, extra_offs = 0;
> -
> -    for (offs = 0x8000; offs <= 0x9800; offs += 0x800)
> -    {
> -        /* if 'CDROM' occurs at position 8, this is a pre-iso9660 cd, and
> -         * the volume label is displaced forward by 8
> -         */
> -        if (SetFilePointer( handle, offs, NULL, FILE_BEGIN ) != offs) break;
> -        if (!ReadFile( handle, buffer, sizeof(buffer), &size, NULL )) break;
> -        if (size != sizeof(buffer)) break;
> -        /* check for non-ISO9660 signature */
> -        if (!memcmp( buffer + 11, "ROM", 3 )) extra_offs = 8;
> -        cur_vd_type = buffer[extra_offs];
> -        if (cur_vd_type == 0xff) /* voldesc set terminator */
> -            break;
> -        if (cur_vd_type > max_vd_type)
> -        {
> -            max_vd_type = cur_vd_type;
> -            best_offs = offs + extra_offs;
> -        }
> -    }
> -    return best_offs;
> -}
> -
> -
> -/***********************************************************************
> - *           read_fat_superblock
> - */
> -static enum fs_type read_fat_superblock( HANDLE handle, BYTE *buff )
> -{
> -    DWORD size;
> -
> -    /* try a fixed disk, with a FAT partition */
> -    if (SetFilePointer( handle, 0, NULL, FILE_BEGIN ) != 0 ||
> -        !ReadFile( handle, buff, SUPERBLOCK_SIZE, &size, NULL ))
> -    {
> -        if (GetLastError() == ERROR_BAD_DEV_TYPE) return FS_UNKNOWN;  /* not a real device */
> -        return FS_ERROR;
> -    }
> -
> -    if (size < SUPERBLOCK_SIZE) return FS_UNKNOWN;
> -
> -    /* FIXME: do really all FAT have their name beginning with
> -     * "FAT" ? (At least FAT12, FAT16 and FAT32 have :)
> -     */
> -    if (!memcmp(buff+0x36, "FAT", 3) || !memcmp(buff+0x52, "FAT", 3))
> -    {
> -        /* guess which type of FAT we have */
> -        int reasonable;
> -        unsigned int sectors,
> -                     sect_per_fat,
> -                     total_sectors,
> -                     num_boot_sectors,
> -                     num_fats,
> -                     num_root_dir_ents,
> -                     bytes_per_sector,
> -                     sectors_per_cluster,
> -                     nclust;
> -        sect_per_fat = GETWORD(buff, 0x16);
> -        if (!sect_per_fat) sect_per_fat = GETLONG(buff, 0x24);
> -        total_sectors = GETWORD(buff, 0x13);
> -        if (!total_sectors)
> -            total_sectors = GETLONG(buff, 0x20);
> -        num_boot_sectors = GETWORD(buff, 0x0e);
> -        num_fats =  buff[0x10];
> -        num_root_dir_ents = GETWORD(buff, 0x11);
> -        bytes_per_sector = GETWORD(buff, 0x0b);
> -        sectors_per_cluster = buff[0x0d];
> -        /* check if the parameters are reasonable and will not cause
> -         * arithmetic errors in the calculation */
> -        reasonable = num_boot_sectors < total_sectors &&
> -                     num_fats < 16 &&
> -                     bytes_per_sector >= 512 && bytes_per_sector % 512 == 0 &&
> -                     sectors_per_cluster >= 1;
> -        if (!reasonable) return FS_UNKNOWN;
> -        sectors =  total_sectors - num_boot_sectors - num_fats * sect_per_fat -
> -            (num_root_dir_ents * 32 + bytes_per_sector - 1) / bytes_per_sector;
> -        nclust = sectors / sectors_per_cluster;
> -        if ((buff[0x42] == 0x28 || buff[0x42] == 0x29) &&
> -                !memcmp(buff+0x52, "FAT", 3)) return FS_FAT32;
> -        if (nclust < 65525)
> -        {
> -            if ((buff[0x26] == 0x28 || buff[0x26] == 0x29) &&
> -                    !memcmp(buff+0x36, "FAT", 3))
> -                return FS_FAT1216;
> -        }
> -    }
> -    return FS_UNKNOWN;
> -}
> -
> -
> -/***********************************************************************
> - *           read_cd_block
> - */
> -static BOOL read_cd_block( HANDLE handle, BYTE *buff, INT offs )
> -{
> -    DWORD size, whence = offs >= 0 ? FILE_BEGIN : FILE_END;
> -
> -    if (SetFilePointer( handle, offs, NULL, whence ) != offs ||
> -        !ReadFile( handle, buff, SUPERBLOCK_SIZE, &size, NULL ) ||
> -        size != SUPERBLOCK_SIZE)
> -        return FALSE;
> -
> -    return TRUE;
> -}
> -
> -
> -/***********************************************************************
> - *           read_cd_superblock
> - */
> -static enum fs_type read_cd_superblock( HANDLE handle, BYTE *buff )
> -{
> -    int i;
> -    DWORD offs;
> -
> -    /* Check UDF first as UDF and ISO9660 structures can coexist on the same medium
> -     *  Starting from sector 16, we may find :
> -     *  - a CD-ROM Volume Descriptor Set (ISO9660) containing one or more Volume Descriptors
> -     *  - an Extended Area (UDF) -- [E] 2/8.3.1 and [U] 2.1.7
> -     *  There is no explicit end so read 16 sectors and then give up */
> -    for( i=16; i<16+16; i++)
> -    {
> -        if (!read_cd_block(handle, buff, i*BLOCK_SIZE))
> -            continue;
> -
> -        /* We are supposed to check "BEA01", "NSR0x" and "TEA01" IDs + verify tag checksum
> -         *  but we assume the volume is well-formatted */
> -        if (!memcmp(&buff[1], "BEA01", 5)) return FS_UDF;
> -    }
> -
> -    offs = find_cdrom_best_voldesc( handle );
> -    if (!offs) return FS_UNKNOWN;
> -
> -    if (!read_cd_block(handle, buff, offs))
> -        return FS_ERROR;
> -
> -    /* check for the iso9660 identifier */
> -    if (!memcmp(&buff[1], "CD001", 5)) return FS_ISO9660;
> -    return FS_UNKNOWN;
> -}
> -
> -
> -/**************************************************************************
> - *                        udf_find_pvd
> - */
> -static BOOL udf_find_pvd( HANDLE handle, BYTE pvd[] )
> -{
> -    unsigned int i;
> -    DWORD offset;
> -    INT locations[] = { 256, -1, -257, 512 };
> -
> -    for(i=0; i<ARRAY_SIZE(locations); i++)
> -    {
> -        if (!read_cd_block(handle, pvd, locations[i]*BLOCK_SIZE))
> -            return FALSE;
> -
> -        /* Tag Identifier of Anchor Volume Descriptor Pointer is 2 -- [E] 3/10.2.1 */
> -        if (pvd[0]==2 && pvd[1]==0)
> -        {
> -            /* Tag location (Uint32) at offset 12, little-endian */
> -            offset  = pvd[20 + 0];
> -            offset |= pvd[20 + 1] << 8;
> -            offset |= pvd[20 + 2] << 16;
> -            offset |= pvd[20 + 3] << 24;
> -            offset *= BLOCK_SIZE;
> -
> -            if (!read_cd_block(handle, pvd, offset))
> -                return FALSE;
> -
> -            /* Check for the Primary Volume Descriptor Tag Id -- [E] 3/10.1.1 */
> -            if (pvd[0]!=1 || pvd[1]!=0)
> -                return FALSE;
> -
> -            /* 8 or 16 bits per character -- [U] 2.1.1 */
> -            if (!(pvd[24]==8 || pvd[24]==16))
> -                return FALSE;
> -
> -            return TRUE;
> -        }
> -    }
> -
> -    return FALSE;
> -}
> -
> -
> -/**************************************************************************
> - *                              get_superblock_label
> - */
> -static void get_superblock_label( const UNICODE_STRING *device, HANDLE handle,
> -                                  enum fs_type type, const BYTE *superblock,
> -                                  WCHAR *label, DWORD len )
> -{
> -    const BYTE *label_ptr = NULL;
> -    DWORD label_len;
> -
> -    switch(type)
> -    {
> -    case FS_ERROR:
> -        label_len = 0;
> -        break;
> -    case FS_UNKNOWN:
> -        get_filesystem_label( device, label, len );
> -        return;
> -    case FS_FAT1216:
> -        label_ptr = superblock + 0x2b;
> -        label_len = 11;
> -        break;
> -    case FS_FAT32:
> -        label_ptr = superblock + 0x47;
> -        label_len = 11;
> -        break;
> -    case FS_ISO9660:
> -        {
> -            BYTE ver = superblock[0x5a];
> -
> -            if (superblock[0x58] == 0x25 && superblock[0x59] == 0x2f &&  /* Unicode ID */
> -                ((ver == 0x40) || (ver == 0x43) || (ver == 0x45)))
> -            { /* yippee, unicode */
> -                unsigned int i;
> -
> -                if (len > 17) len = 17;
> -                for (i = 0; i < len-1; i++)
> -                    label[i] = (superblock[40+2*i] << 8) | superblock[41+2*i];
> -                label[i] = 0;
> -                while (i && label[i-1] == ' ') label[--i] = 0;
> -                return;
> -            }
> -            label_ptr = superblock + 40;
> -            label_len = 32;
> -            break;
> -        }
> -    case FS_UDF:
> -        {
> -            BYTE pvd[BLOCK_SIZE];
> -
> -            if(!udf_find_pvd(handle, pvd))
> -            {
> -                label_len = 0;
> -                break;
> -            }
> -
> -            /* [E] 3/10.1.4 and [U] 2.1.1 */
> -            if(pvd[24]==8)
> -            {
> -                label_ptr = pvd + 24 + 1;
> -                label_len = pvd[24+32-1];
> -                break;
> -            }
> -            else
> -            {
> -                unsigned int i;
> -
> -                label_len = 1 + pvd[24+32-1];
> -                for(i=0; i<label_len && i<len; i+=2)
> -                    label[i/2]  = (pvd[24+1 +i] << 8) | pvd[24+1 +i+1];
> -                label[label_len] = 0;
> -                return;
> -            }
> -        }
> -    }
> -    if (label_len) RtlMultiByteToUnicodeN( label, (len-1) * sizeof(WCHAR),
> -                                           &label_len, (LPCSTR)label_ptr, label_len );
> -    label_len /= sizeof(WCHAR);
> -    label[label_len] = 0;
> -    while (label_len && label[label_len-1] == ' ') label[--label_len] = 0;
> -}
> -
> -
> -/**************************************************************************
> - *                              udf_find_fsd_sector
> - */
> -static int udf_find_fsd_sector( HANDLE handle, BYTE block[] )
> -{
> -    int i, PVD_sector, PD_sector, PD_length;
> -
> -    if(!udf_find_pvd(handle,block))
> -        goto default_sector;
> -
> -    /* Retrieve the tag location of the PVD -- [E] 3/7.2 */
> -    PVD_sector  = block[12 + 0];
> -    PVD_sector |= block[12 + 1] << 8;
> -    PVD_sector |= block[12 + 2] << 16;
> -    PVD_sector |= block[12 + 3] << 24;
> -
> -    /* Find the Partition Descriptor */
> -    for(i=PVD_sector+1; ; i++)
> -    {
> -        if(!read_cd_block(handle, block, i*BLOCK_SIZE))
> -            goto default_sector;
> -
> -        /* Partition Descriptor Tag Id -- [E] 3/10.5.1 */
> -        if(block[0]==5 && block[1]==0)
> -            break;
> -
> -        /* Terminating Descriptor Tag Id -- [E] 3/10.9.1 */
> -        if(block[0]==8 && block[1]==0)
> -            goto default_sector;
> -    }
> -
> -    /* Find the partition starting location -- [E] 3/10.5.8 */
> -    PD_sector  = block[188 + 0];
> -    PD_sector |= block[188 + 1] << 8;
> -    PD_sector |= block[188 + 2] << 16;
> -    PD_sector |= block[188 + 3] << 24;
> -
> -    /* Find the partition length -- [E] 3/10.5.9 */
> -    PD_length  = block[192 + 0];
> -    PD_length |= block[192 + 1] << 8;
> -    PD_length |= block[192 + 2] << 16;
> -    PD_length |= block[192 + 3] << 24;
> -
> -    for(i=PD_sector; i<PD_sector+PD_length; i++)
> -    {
> -        if(!read_cd_block(handle, block, i*BLOCK_SIZE))
> -            goto default_sector;
> -
> -        /* File Set Descriptor Tag Id -- [E] 3/14.1.1 */
> -        if(block[0]==0 && block[1]==1)
> -            return i;
> -    }
> -
> -default_sector:
> -    WARN("FSD sector not found, serial may be incorrect\n");
> -    return 257;
> -}
> -
> -
> -/**************************************************************************
> - *                              get_superblock_serial
> - */
> -static DWORD get_superblock_serial( const UNICODE_STRING *device, HANDLE handle,
> -                                    enum fs_type type, const BYTE *superblock )
> -{
> -    int FSD_sector;
> -    BYTE block[BLOCK_SIZE];
> -
> -    switch(type)
> -    {
> -    case FS_ERROR:
> -        break;
> -    case FS_UNKNOWN:
> -        return get_filesystem_serial( device );
> -    case FS_FAT1216:
> -        return GETLONG( superblock, 0x27 );
> -    case FS_FAT32:
> -        return GETLONG( superblock, 0x33 );
> -    case FS_UDF:
> -        FSD_sector = udf_find_fsd_sector(handle, block);
> -        if (!read_cd_block(handle, block, FSD_sector*BLOCK_SIZE))
> -            break;
> -        superblock = block;
> -        /* fallthrough */
> -    case FS_ISO9660:
> -        {
> -            BYTE sum[4];
> -            int i;
> -
> -            sum[0] = sum[1] = sum[2] = sum[3] = 0;
> -            for (i = 0; i < 2048; i += 4)
> -            {
> -                /* DON'T optimize this into DWORD !! (breaks overflow) */
> -                sum[0] += superblock[i+0];
> -                sum[1] += superblock[i+1];
> -                sum[2] += superblock[i+2];
> -                sum[3] += superblock[i+3];
> -            }
> -            /*
> -             * OK, another braindead one... argh. Just believe it.
> -             * Me$$ysoft chose to reverse the serial number in NT4/W2K.
> -             * It's true and nobody will ever be able to change it.
> -             */
> -            if ((GetVersion() & 0x80000000) || type == FS_UDF)
> -                return (sum[3] << 24) | (sum[2] << 16) | (sum[1] << 8) | sum[0];
> -            else
> -                return (sum[0] << 24) | (sum[1] << 16) | (sum[2] << 8) | sum[3];
> -        }
> -    }
> -    return 0;
> -}
> -
> -
> -/**************************************************************************
> - *                              get_audiocd_serial
> - */
> -static DWORD get_audiocd_serial( const CDROM_TOC *toc )
> -{
> -    DWORD serial = 0;
> -    int i;
> -
> -    for (i = 0; i <= toc->LastTrack - toc->FirstTrack; i++)
> -        serial += ((toc->TrackData[i].Address[1] << 16) |
> -                   (toc->TrackData[i].Address[2] << 8) |
> -                   toc->TrackData[i].Address[3]);
> -
> -    /*
> -     * dwStart, dwEnd collect the beginning and end of the disc respectively, in
> -     * frames.
> -     * There it is collected for correcting the serial when there are less than
> -     * 3 tracks.
> -     */
> -    if (toc->LastTrack - toc->FirstTrack + 1 < 3)
> -    {
> -        DWORD dwStart = FRAME_OF_TOC(toc, toc->FirstTrack);
> -        DWORD dwEnd = FRAME_OF_TOC(toc, toc->LastTrack + 1);
> -        serial += dwEnd - dwStart;
> -    }
> -    return serial;
> -}
> -
>
> /***********************************************************************
>   *           GetVolumeInformationW   (kernelbase.@)
> @@ -656,9 +162,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetVolumeInformationW( LPCWSTR root, LPWSTR label,
>      UNICODE_STRING nt_name;
>      IO_STATUS_BLOCK io;
>      OBJECT_ATTRIBUTES attr;
> -    FILE_FS_DEVICE_INFORMATION info;
>      unsigned int i;
> -    enum fs_type type = FS_UNKNOWN;
>      BOOL ret = FALSE;
>
> if (!root) root = L"\\";
> @@ -681,8 +185,6 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetVolumeInformationW( LPCWSTR root, LPWSTR label,
>          goto done;
>      }
>
> -    /* try to open the device */
> -
>      attr.Length = sizeof(attr);
>      attr.RootDirectory = 0;
>      attr.Attributes = OBJ_CASE_INSENSITIVE;
> @@ -690,99 +192,14 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetVolumeInformationW( LPCWSTR root, LPWSTR label,
>      attr.SecurityDescriptor = NULL;
>      attr.SecurityQualityOfService = NULL;
>
> -    nt_name.Length -= sizeof(WCHAR);  /* without trailing slash */
> -    status = NtOpenFile( &handle, GENERIC_READ | SYNCHRONIZE, &attr, &io, FILE_SHARE_READ | FILE_SHARE_WRITE,
> -                         FILE_NON_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT );
> -    nt_name.Length += sizeof(WCHAR);
> -
> -    if (status == STATUS_SUCCESS)
> -    {
> -        BYTE superblock[SUPERBLOCK_SIZE];
> -        CDROM_TOC toc;
> -        DWORD br;
> -
> -        /* check for audio CD */
> -        /* FIXME: we only check the first track for now */
> -        if (DeviceIoControl( handle, IOCTL_CDROM_READ_TOC, NULL, 0, &toc, sizeof(toc), &br, 0 ))
> -        {
> -            if (!(toc.TrackData[0].Control & 0x04))  /* audio track */
> -            {
> -                TRACE( "%s: found audio CD\n", debugstr_w(nt_name.Buffer) );
> -                if (label) lstrcpynW( label, L"Audio CD", label_len );
> -                if (serial) *serial = get_audiocd_serial( &toc );
> -                CloseHandle( handle );
> -                type = FS_ISO9660;
> -                goto fill_fs_info;
> -            }
> -            type = read_cd_superblock( handle, superblock );
> -        }
> -        else
> -        {
> -            type = read_fat_superblock( handle, superblock );
> -            if (type == FS_UNKNOWN) type = read_cd_superblock( handle, superblock );
> -        }
> -        TRACE( "%s: found fs type %d\n", debugstr_w(nt_name.Buffer), type );
> -        if (type == FS_ERROR)
> -        {
> -            CloseHandle( handle );
> -            goto done;
> -        }
> -
> -        if (label && label_len) get_superblock_label( &nt_name, handle, type, superblock, label, label_len );
> -        if (serial) *serial = get_superblock_serial( &nt_name, handle, type, superblock );
> -        CloseHandle( handle );
> -        goto fill_fs_info;
> -    }
> -    else
> -    {
> -        TRACE( "cannot open device %s: %x\n", debugstr_w(nt_name.Buffer), status );
> -        if (status == STATUS_ACCESS_DENIED)
> -            MESSAGE( "wine: Read access denied for device %s, FS volume label and serial are not available.\n", debugstr_w(nt_name.Buffer) );
> -    }
> -    /* we couldn't open the device, fallback to default strategy */
> -
> -    if (!set_ntstatus( NtOpenFile( &handle, SYNCHRONIZE, &attr, &io, 0,
> -                                   FILE_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT )))
> -        goto done;
> -
> -    status = NtQueryVolumeInformationFile( handle, &io, &info, sizeof(info), FileFsDeviceInformation );
> -    NtClose( handle );
> +    status = NtOpenFile( &handle, SYNCHRONIZE | FILE_LIST_DIRECTORY, &attr, &io,
> +                         FILE_SHARE_READ | FILE_SHARE_WRITE,
> +                         FILE_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT );

Why FILE_LIST_DIRECTORY?
> if (!set_ntstatus( status )) goto done;
>
> -    if (info.DeviceType == FILE_DEVICE_CD_ROM_FILE_SYSTEM) type = FS_ISO9660;
> -
> -    if (label && label_len) get_filesystem_label( &nt_name, label, label_len );
> -    if (serial) *serial = get_filesystem_serial( &nt_name );
> -
> -fill_fs_info:  /* now fill in the information that depends on the file system type */
> -
> -    switch(type)
> -    {
> -    case FS_ISO9660:
> -        if (fsname) lstrcpynW( fsname, L"CDFS", fsname_len );
> -        if (filename_len) *filename_len = 221;
> -        if (flags) *flags = FILE_READ_ONLY_VOLUME;
> -        break;
> -    case FS_UDF:
> -        if (fsname) lstrcpynW( fsname, L"UDF", fsname_len );
> -        if (filename_len) *filename_len = 255;
> -        if (flags)
> -            *flags = FILE_READ_ONLY_VOLUME | FILE_UNICODE_ON_DISK | FILE_CASE_SENSITIVE_SEARCH;
> -        break;
> -    case FS_FAT1216:
> -        if (fsname) lstrcpynW( fsname, L"FAT", fsname_len );
> -    case FS_FAT32:
> -        if (type == FS_FAT32 && fsname) lstrcpynW( fsname, L"FAT32", fsname_len );
> -        if (filename_len) *filename_len = 255;
> -        if (flags) *flags = FILE_CASE_PRESERVED_NAMES;  /* FIXME */
> -        break;
> -    default:
> -        if (fsname) lstrcpynW( fsname, L"NTFS", fsname_len );
> -        if (filename_len) *filename_len = 255;
> -        if (flags) *flags = FILE_CASE_PRESERVED_NAMES | FILE_PERSISTENT_ACLS;
> -        break;
> -    }
> -    ret = TRUE;
> +    ret = GetVolumeInformationByHandleW( handle, label, label_len, serial, filename_len, flags,
> +                                         fsname, fsname_len );
> +    NtClose( handle );
>
> done:
>      RtlFreeUnicodeString( &nt_name );
>
> -- 
> 2.17.1

[Erich E. Hoover]

On Wed, Feb 10, 2021 at 9:37 PM Zebediah Figura (she/her)
<zfigura(a)codeweavers.com> wrote:
> Just a couple small nitpicks...
>
> On 2/6/21 12:26 PM, Erich E. Hoover wrote:
> > From: "Erich E. Hoover" <erich.e.hoover(a)gmail.com>
> > Subject: [PATCH v6 5/5] kernelbase: Reimplement GetVolumeInformation on top of GetVolumeInformationByHandle.
> > Message-Id: <CAEU2+vrAcFN5oXJvunL0umLXW4W0je0K41cEfEtPkAkDre=StQ(a)mail.gmail.com>
> > Date: Sat, 6 Feb 2021 11:26:05 -0700
> > ...
> > @@ -1522,6 +1522,8 @@ static void test_GetVolumeInformationByHandle(void)
> >      FILE_FS_VOLUME_INFORMATION *volume_info = (void *)buffer;
> >      DWORD serial, filename_len, flags;
> >      WCHAR label[20], fsname[20];
> > +    char Root_Slash[]="C:\\";
>
> Why is this a local variable?

This is copied from test_GetVolumeInformationA, but it looks like I
missed adding this block:
    /* get windows drive letter and update strings for testing */
    result = GetWindowsDirectoryA(windowsdir, sizeof(windowsdir));
    ok(result < sizeof(windowsdir), "windowsdir is abnormally long!\n");
    ok(result != 0, "GetWindowsDirectory: error %d\n", GetLastError());
    Root_Colon[0] = windowsdir[0];
    Root_Slash[0] = windowsdir[0];
I'll add this missing block in so that the test always uses the
correct drive letter...
> > ...
> > -    nt_name.Length -= sizeof(WCHAR);  /* without trailing slash */
> > -    status = NtOpenFile( &handle, GENERIC_READ | SYNCHRONIZE, &attr, &io, FILE_SHARE_READ | FILE_SHARE_WRITE,
> > -                         FILE_NON_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT );
> > -    nt_name.Length += sizeof(WCHAR);
> > ...
> > -    status = NtQueryVolumeInformationFile( handle, &io, &info, sizeof(info), FileFsDeviceInformation );
> > -    NtClose( handle );
> > +    status = NtOpenFile( &handle, SYNCHRONIZE | FILE_LIST_DIRECTORY, &attr, &io,
> > +                         FILE_SHARE_READ | FILE_SHARE_WRITE,
> > +                         FILE_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT );
>
> Why FILE_LIST_DIRECTORY?

I honestly have no idea, I thought I copied from the original
NtOpenFile (above).  It's possible that changed at some point, but it
could also be that I copied it from somewhere else or that it's a
debugging remnant (it's been a while since I originally put this
together).
> > ...