[PATCH 0/3] MR5976: Acquire filesystem labels from dbus
From: Alfred Agrell <floating(a)muncher.se> --- dlls/ntdll/locale.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/locale.c b/dlls/ntdll/locale.c index 545cc628909..ebc4b3befb6 100644 --- a/dlls/ntdll/locale.c +++ b/dlls/ntdll/locale.c @@ -950,14 +950,15 @@ NTSTATUS WINAPI RtlUTF8ToUnicodeN( WCHAR *dst, DWORD dstlen, DWORD *reslen, cons NTSTATUS status; if (!src) return STATUS_INVALID_PARAMETER_4; - if (!reslen) return STATUS_INVALID_PARAMETER; + if (!reslen && !dst) return STATUS_INVALID_PARAMETER; if (!dst) status = utf8_mbstowcs_size( src, srclen, &ret ); else status = utf8_mbstowcs( dst, dstlen / sizeof(WCHAR), &ret, src, srclen ); - *reslen = ret * sizeof(WCHAR); + if (reslen) + *reslen = ret * sizeof(WCHAR); return status; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5976
From: Alfred Agrell <floating(a)muncher.se> --- dlls/mountmgr.sys/dbus.c | 24 +++++++++++++++--------- dlls/mountmgr.sys/device.c | 14 ++++++++------ dlls/mountmgr.sys/diskarb.c | 6 +++--- dlls/mountmgr.sys/mountmgr.c | 6 +++--- dlls/mountmgr.sys/mountmgr.h | 4 ++-- dlls/mountmgr.sys/unixlib.c | 4 +++- dlls/mountmgr.sys/unixlib.h | 4 +++- 7 files changed, 37 insertions(+), 25 deletions(-) diff --git a/dlls/mountmgr.sys/dbus.c b/dlls/mountmgr.sys/dbus.c index 6fe6984c218..52b5c203660 100644 --- a/dlls/mountmgr.sys/dbus.c +++ b/dlls/mountmgr.sys/dbus.c @@ -191,6 +191,7 @@ static void udisks_new_device( const char *udi ) const char *device = NULL; const char *mount_point = NULL; const char *type = NULL; + const char *label = NULL; GUID guid, *guid_ptr = NULL; int removable = FALSE; enum device_type drive_type = DEVICE_UNKNOWN; @@ -227,6 +228,8 @@ static void udisks_new_device( const char *udi ) p_dbus_message_iter_get_basic( &variant, &removable ); else if (!strcmp( name, "IdType" )) p_dbus_message_iter_get_basic( &variant, &type ); + else if (!strcmp( name, "IdLabel" )) + p_dbus_message_iter_get_basic( &variant, &label ); else if (!strcmp( name, "DriveMediaCompatibility" )) drive_type = udisks_parse_media_compatibility( &variant ); else if (!strcmp( name, "DeviceMountPaths" )) @@ -245,9 +248,9 @@ static void udisks_new_device( const char *udi ) } } - TRACE( "udi %s device %s mount point %s uuid %s type %s removable %u\n", + TRACE( "udi %s device %s mount point %s uuid %s type %s label %s removable %u\n", debugstr_a(udi), debugstr_a(device), debugstr_a(mount_point), - debugstr_guid(guid_ptr), debugstr_a(type), removable ); + debugstr_guid(guid_ptr), debugstr_a(type), debugstr_a(label), removable ); if (type) { @@ -265,8 +268,8 @@ static void udisks_new_device( const char *udi ) if (device) { - if (removable) queue_device_op( ADD_DOS_DEVICE, udi, device, mount_point, drive_type, guid_ptr, NULL, NULL ); - else if (guid_ptr) queue_device_op( ADD_VOLUME, udi, device, mount_point, DEVICE_HARDDISK_VOL, guid_ptr, NULL, NULL ); + if (removable) queue_device_op( ADD_DOS_DEVICE, udi, device, mount_point, drive_type, guid_ptr, NULL, label, NULL ); + else if (guid_ptr) queue_device_op( ADD_VOLUME, udi, device, mount_point, DEVICE_HARDDISK_VOL, guid_ptr, NULL, label, NULL ); } p_dbus_message_unref( reply ); @@ -276,7 +279,7 @@ static void udisks_new_device( const char *udi ) static void udisks_removed_device( const char *udi ) { TRACE( "removed %s\n", wine_dbgstr_a(udi) ); - queue_device_op( REMOVE_DEVICE, udi, NULL, NULL, 0, NULL, NULL, NULL ); + queue_device_op( REMOVE_DEVICE, udi, NULL, NULL, 0, NULL, NULL, NULL, NULL ); } /* UDisks callback for changed device */ @@ -366,6 +369,7 @@ static void udisks2_add_device( const char *udi, DBusMessageIter *dict, DBusMess const char *type = NULL; const char *drive = NULL; const char *id = NULL; + const char *label = NULL; GUID guid, *guid_ptr = NULL; const char *iface, *name; int removable = FALSE; @@ -396,6 +400,8 @@ static void udisks2_add_device( const char *udi, DBusMessageIter *dict, DBusMess device = udisks2_string_from_array( &variant ); else if (!strcmp( name, "IdType" )) p_dbus_message_iter_get_basic( &variant, &type ); + else if (!strcmp( name, "IdLabel" )) + p_dbus_message_iter_get_basic( &variant, &label ); else if (!strcmp( name, "Drive" )) { p_dbus_message_iter_get_basic( &variant, &drive ); @@ -414,9 +420,9 @@ static void udisks2_add_device( const char *udi, DBusMessageIter *dict, DBusMess } } - TRACE( "udi %s device %s mount point %s uuid %s type %s removable %u\n", + TRACE( "udi %s device %s mount point %s uuid %s type %s label %s removable %u\n", debugstr_a(udi), debugstr_a(device), debugstr_a(mount_point), - debugstr_guid(guid_ptr), debugstr_a(type), removable ); + debugstr_guid(guid_ptr), debugstr_a(type), debugstr_a(label), removable ); if (type) { @@ -433,8 +439,8 @@ static void udisks2_add_device( const char *udi, DBusMessageIter *dict, DBusMess } if (device) { - if (removable) queue_device_op( ADD_DOS_DEVICE, udi, device, mount_point, drive_type, guid_ptr, id, NULL ); - else if (guid_ptr) queue_device_op( ADD_VOLUME, udi, device, mount_point, DEVICE_HARDDISK_VOL, guid_ptr, id, NULL ); + if (removable) queue_device_op( ADD_DOS_DEVICE, udi, device, mount_point, drive_type, guid_ptr, id, label, NULL ); + else if (guid_ptr) queue_device_op( ADD_VOLUME, udi, device, mount_point, DEVICE_HARDDISK_VOL, guid_ptr, id, label, NULL ); } } diff --git a/dlls/mountmgr.sys/device.c b/dlls/mountmgr.sys/device.c index 48ee8d9189a..9ed676883c2 100644 --- a/dlls/mountmgr.sys/device.c +++ b/dlls/mountmgr.sys/device.c @@ -949,7 +949,7 @@ static void set_dos_devices_disk_serial( struct disk_device *device ) /* change the information for an existing volume */ static NTSTATUS set_volume_info( struct volume *volume, struct dos_drive *drive, const char *device, const char *mount_point, enum device_type type, const GUID *guid, - const char *disk_serial ) + const char *label, const char *disk_serial ) { void *id = NULL; unsigned int id_len = 0; @@ -995,6 +995,8 @@ static NTSTATUS set_volume_info( struct volume *volume, struct dos_drive *drive, get_filesystem_label( volume ); get_filesystem_serial( volume ); } + if (label && label[0] && !volume->label[0]) + RtlUTF8ToUnicodeN(volume->label, ARRAY_SIZE(volume->label), NULL, label, strlen(label)+1); TRACE("fs_type %#x, label %s, serial %08lx\n", volume->fs_type, debugstr_w(volume->label), volume->serial); @@ -1098,7 +1100,7 @@ static void create_drive_devices(void) { /* don't reset uuid if we used an existing volume */ const GUID *guid = volume ? NULL : get_default_uuid(i); - set_volume_info( drive->volume, drive, device, link, drive_type, guid, NULL ); + set_volume_info( drive->volume, drive, device, link, drive_type, guid, NULL, NULL ); } if (volume) release_volume( volume ); } @@ -1198,7 +1200,7 @@ static void create_scsi_entry( struct volume *volume, const struct scsi_info *in /* create a new disk volume */ NTSTATUS add_volume( const char *udi, const char *device, const char *mount_point, enum device_type type, const GUID *guid, const char *disk_serial, - const struct scsi_info *scsi_info ) + const char *label, const struct scsi_info *scsi_info ) { struct volume *volume; NTSTATUS status = STATUS_SUCCESS; @@ -1219,7 +1221,7 @@ NTSTATUS add_volume( const char *udi, const char *device, const char *mount_poin else status = create_volume( udi, type, &volume ); found: - if (!status) status = set_volume_info( volume, NULL, device, mount_point, type, guid, disk_serial ); + if (!status) status = set_volume_info( volume, NULL, device, mount_point, type, guid, label, disk_serial ); if (!status && scsi_info) create_scsi_entry( volume, scsi_info ); if (volume) release_volume( volume ); LeaveCriticalSection( &device_section ); @@ -1248,7 +1250,7 @@ NTSTATUS remove_volume( const char *udi ) /* create a new dos drive */ NTSTATUS add_dos_device( int letter, const char *udi, const char *device, const char *mount_point, enum device_type type, const GUID *guid, - const struct scsi_info *scsi_info ) + const char *label, const struct scsi_info *scsi_info ) { HKEY hkey; NTSTATUS status = STATUS_SUCCESS; @@ -1305,7 +1307,7 @@ found: struct set_dosdev_symlink_params params = { dosdev, mount_point }; MOUNTMGR_CALL( set_dosdev_symlink, ¶ms ); } - set_volume_info( volume, drive, device, mount_point, type, guid, NULL ); + set_volume_info( volume, drive, device, mount_point, type, guid, label, NULL ); TRACE( "added device %c: udi %s for %s on %s type %u\n", 'a' + drive->drive, wine_dbgstr_a(udi), wine_dbgstr_a(device), diff --git a/dlls/mountmgr.sys/diskarb.c b/dlls/mountmgr.sys/diskarb.c index d33f6c60afb..5a46da7f599 100644 --- a/dlls/mountmgr.sys/diskarb.c +++ b/dlls/mountmgr.sys/diskarb.c @@ -180,9 +180,9 @@ static void appeared_callback( DADiskRef disk, void *context ) } if (removable) - queue_device_op( ADD_DOS_DEVICE, device, device, mount_point, type, guid_ptr, NULL, &scsi_info ); + queue_device_op( ADD_DOS_DEVICE, device, device, mount_point, type, guid_ptr, NULL, NULL, &scsi_info ); else - if (guid_ptr) queue_device_op( ADD_VOLUME, device, device, mount_point, DEVICE_HARDDISK_VOL, guid_ptr, NULL, &scsi_info ); + if (guid_ptr) queue_device_op( ADD_VOLUME, device, device, mount_point, DEVICE_HARDDISK_VOL, guid_ptr, NULL, NULL, &scsi_info ); done: CFRelease( dict ); @@ -208,7 +208,7 @@ static void disappeared_callback( DADiskRef disk, void *context ) TRACE( "got unmount notification for '%s'\n", device ); - queue_device_op( REMOVE_DEVICE, device, NULL, NULL, 0, NULL, NULL, NULL ); + queue_device_op( REMOVE_DEVICE, device, NULL, NULL, 0, NULL, NULL, NULL, NULL ); done: CFRelease( dict ); diff --git a/dlls/mountmgr.sys/mountmgr.c b/dlls/mountmgr.sys/mountmgr.c index ad3c034fe0f..8d42fb36bad 100644 --- a/dlls/mountmgr.sys/mountmgr.c +++ b/dlls/mountmgr.sys/mountmgr.c @@ -257,7 +257,7 @@ static NTSTATUS define_unix_drive( const void *in_buff, SIZE_T insize ) case DRIVE_RAMDISK: type = DEVICE_RAMDISK; break; case DRIVE_FIXED: type = DEVICE_HARDDISK_VOL; break; } - return add_dos_device( letter - 'a', NULL, device, mount_point, type, NULL, NULL ); + return add_dos_device( letter - 'a', NULL, device, mount_point, type, NULL, NULL, NULL ); } else { @@ -444,11 +444,11 @@ static void CALLBACK device_op( ULONG_PTR arg1, ULONG_PTR arg2, ULONG_PTR arg3 ) { case ADD_DOS_DEVICE: add_dos_device( -1, info.udi, info.device, info.mount_point, - info.type, info.guid, info.scsi_info ); + info.type, info.guid, info.label, info.scsi_info ); break; case ADD_VOLUME: add_volume( info.udi, info.device, info.mount_point, DEVICE_HARDDISK_VOL, - info.guid, info.serial, info.scsi_info ); + info.guid, info.serial, info.label, info.scsi_info ); break; case REMOVE_DEVICE: if (!remove_dos_device( -1, info.udi )) remove_volume( info.udi ); diff --git a/dlls/mountmgr.sys/mountmgr.h b/dlls/mountmgr.sys/mountmgr.h index b1de312dffb..ae6927335b1 100644 --- a/dlls/mountmgr.sys/mountmgr.h +++ b/dlls/mountmgr.sys/mountmgr.h @@ -90,11 +90,11 @@ struct scsi_info extern NTSTATUS add_volume( const char *udi, const char *device, const char *mount_point, enum device_type type, const GUID *guid, const char *disk_serial, - const struct scsi_info *info ); + const char *label, const struct scsi_info *info ); extern NTSTATUS remove_volume( const char *udi ); extern NTSTATUS add_dos_device( int letter, const char *udi, const char *device, const char *mount_point, enum device_type type, const GUID *guid, - const struct scsi_info *info ); + const char *label, const struct scsi_info *info ); extern NTSTATUS remove_dos_device( int letter, const char *udi ); extern NTSTATUS query_unix_drive( void *buff, SIZE_T insize, SIZE_T outsize, IO_STATUS_BLOCK *iosb ); diff --git a/dlls/mountmgr.sys/unixlib.c b/dlls/mountmgr.sys/unixlib.c index 47c42d939c6..36f9f82dad8 100644 --- a/dlls/mountmgr.sys/unixlib.c +++ b/dlls/mountmgr.sys/unixlib.c @@ -133,7 +133,7 @@ static void detect_devices( const char **paths, char *names, ULONG size ) void queue_device_op( enum device_op op, const char *udi, const char *device, const char *mount_point, enum device_type type, const GUID *guid, - const char *serial, const struct scsi_info *scsi_info ) + const char *serial, const char *label, const struct scsi_info *scsi_info ) { struct device_info *info; char *str, *end; @@ -152,6 +152,7 @@ void queue_device_op( enum device_op op, const char *udi, const char *device, ADD_STR(device); ADD_STR(mount_point); ADD_STR(serial); + ADD_STR(label); #undef ADD_STR if (guid) { @@ -188,6 +189,7 @@ static NTSTATUS dequeue_device_op( void *args ) if (dst->device) dst->device = (char *)dst + (src->device - (char *)src); if (dst->mount_point) dst->mount_point = (char *)dst + (src->mount_point - (char *)src); if (dst->serial) dst->serial = (char *)dst + (src->serial - (char *)src); + if (dst->label) dst->label = (char *)dst + (src->label - (char *)src); if (dst->guid) dst->guid = &dst->guid_buffer; if (dst->scsi_info) dst->scsi_info = &dst->scsi_buffer; diff --git a/dlls/mountmgr.sys/unixlib.h b/dlls/mountmgr.sys/unixlib.h index 41f64251e22..b40b323ac80 100644 --- a/dlls/mountmgr.sys/unixlib.h +++ b/dlls/mountmgr.sys/unixlib.h @@ -36,6 +36,7 @@ struct device_info const char *device; const char *mount_point; const char *serial; + const char *label; GUID *guid; struct scsi_info *scsi_info; @@ -179,7 +180,8 @@ enum mountmgr_funcs extern void queue_device_op( enum device_op op, const char *udi, const char *device, const char *mount_point, enum device_type type, const GUID *guid, - const char *disk_serial, const struct scsi_info *info ); + const char *disk_serial, const char *label, + const struct scsi_info *info ); extern void run_dbus_loop(void); extern void run_diskarbitration_loop(void); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5976
From: Alfred Agrell <floating(a)muncher.se> --- dlls/kernelbase/volume.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/dlls/kernelbase/volume.c b/dlls/kernelbase/volume.c index 944c275460c..d3c10d3683e 100644 --- a/dlls/kernelbase/volume.c +++ b/dlls/kernelbase/volume.c @@ -191,19 +191,8 @@ 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) - { - TRACE( "cannot open device %s: %lx\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) ); - status = NtOpenFile( &handle, SYNCHRONIZE, &attr, &io, 0, - FILE_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT ); - } + status = NtOpenFile( &handle, SYNCHRONIZE, &attr, &io, 0, + FILE_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT ); if (!set_ntstatus( status )) goto done; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5976
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=146760 Your paranoid android. === debian11b (64 bit WoW report) === user32: input.c:5880: Test failed: got pos (49,51)
Paul Gofman (@gofman) commented about dlls/kernelbase/volume.c:
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 );
The device is currently opened for read access though which seem to contradict to patch subject. Probably more important, there seems to be other information read through NtQueryVolumeInformationFile( FileFsAttributeInformation ) which has a fallback with the reading from the provided file handle if mountmgr path failed. If for some reason (doubt so?) it is deemed unneeded now that should be probably first removed from NtQueryVolumeInformationFile implementation? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_74951
On Tue Jul 2 22:30:57 2024 +0000, Paul Gofman wrote:
The device is currently opened for read access though which seem to contradict to patch subject. Probably more important, there seems to be other information read through NtQueryVolumeInformationFile( FileFsAttributeInformation ) which has a fallback with the reading from the provided file handle if mountmgr path failed. If for some reason (doubt so?) it is deemed unneeded now that should be probably first removed from NtQueryVolumeInformationFile implementation? In the other aspect, removing this functionality looks a bit orthogonal to adding the label info from dbus?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_74952
Paul Gofman (@gofman) commented about dlls/ntdll/locale.c:
NTSTATUS status;
if (!src) return STATUS_INVALID_PARAMETER_4; - if (!reslen) return STATUS_INVALID_PARAMETER; + if (!reslen && !dst) return STATUS_INVALID_PARAMETER;
This could probably use a test? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_74953
On Tue Jul 2 22:39:22 2024 +0000, Paul Gofman wrote:
In the other aspect, removing this functionality looks a bit orthogonal to adding the label info from dbus? ...uh. You're right. I misread FILE_SHARE_WRITE as opening for write.
My main goal (for this entire MR) is to shut up that MESSAGE for Z:. Judging by commit 4907f262, the goal is to print that only if the device is a mounted ISO file or an actual optical drive. Let's check for specifically that. It's somewhat orthogonal, yes, but that message is now likely to be printed even if the device has a label, so I feel this is a good place to tone it down. (It was already possible, if the device had a .windows-label file, but pretty much nobody knew about that.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_74954
On Tue Jul 2 22:40:49 2024 +0000, Paul Gofman wrote:
This could probably use a test? Looks like I shouldn't call this function at all, the rest of mountmgr uses RtlMultiByteToUnicodeN.
Which already handles missing dst correctly, making this patch unnecessary and irrelevant. I'll drop it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5976#note_74956
participants (4)
-
Alfred Agrell -
Alfred Agrell (＠Alcaro)
-
Marvin -
Paul Gofman (＠gofman)