Re: [PATCH] Cygwin: Add /dev/disk/by-label and /dev/disk/by-uuid symlinks

[Christian Franke <Christian.Franke@t-online.de>]

[-- Attachment #1: Type: text/plain, Size: 2268 bytes --]

Corinna Vinschen wrote:
> On Nov 20 10:40, Corinna Vinschen wrote:
>> Hi Christian,
>>
>> This puzzles me:
>>
>> On Nov 17 21:25, Christian Franke wrote:
>>> @@ -610,7 +607,7 @@ get_by_id_table (by_id_entry * &table, fhandler_dev_disk::dev_disk_location loc)
>>>     if (!table)
>>>       return (errno_set ? -1 : 0);
>>>   
>>> -  /* Sort by name and remove duplicates. */
>>> +  /* Sort by name and mark duplicates. */
>>>     qsort (table, table_size, sizeof (*table), by_id_compare_name);
>>>     for (unsigned i = 0; i < table_size; i++)
>> by_id_compare_name only compars the actual names...
>>
>>>       {
>>> @@ -619,12 +616,13 @@ get_by_id_table (by_id_entry * &table, fhandler_dev_disk::dev_disk_location loc)
>>>   	j++;
>>>         if (j == i + 1)
>>>   	continue;
>>> -      /* Duplicate(s) found, remove all entries with this name. */
>>> -      debug_printf ("removing duplicates %d-%d: '%s'", i, j - 1, table[i].name);
>>> -      if (j < table_size)
>>> -	memmove (table + i, table + j, (table_size - j) * sizeof (*table));
>>> -      table_size -= j - i;
>>> -      i--;
>>> +      /* Duplicate(s) found, append "#N" to all entries.  This never
>> ...but the names are identical.  So the *order* within the identically
>> named entries depends on qsort's reshuffling of table
>> entries.  Which in turn depends on outside factors like number of table
>> entries and the ultimate position of the identical entries within the
>> ordered table.
>>
>> Having said that, I don't see how adding ordinals to the names can be
>> unambiguous.  AFAICS, the numbers may change by just adding another
>> disk (USB Stick) to the system...
> Oops, that's not exactly what I was trying to say, sorry.
>
> The problem is not adding ordinals to the name, AFAICS, the problem is
> that the sorting function by_id_compare_name is not up to the task to
> make sure the order is unambiguous within the entries of identical name.

That's correct, thanks for catching. qsort is not a stable sort. 
Changing drives outside the duplicate range may also change the order 
within the range. Could be fixed by a lexicographic compare of {name, 
drive, part}.

I'll provide a new patch soon. For now, I attached the unrelated but 
important part of the dropped patch.

Christian


[-- Attachment #2: 0001-Cygwin-dev-disk-by-uuid-Fix-NTFS-serial-number-print.patch --]
[-- Type: text/plain, Size: 1015 bytes --]

From 64c8a0eb3ead91de36ee65516483e40cf7f49446 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Mon, 20 Nov 2023 15:40:42 +0100
Subject: [PATCH] Cygwin: /dev/disk/by-uuid: Fix NTFS serial number print
 format

Signed-off-by: Christian Franke <christian.franke@t-online.de>
---
 winsup/cygwin/fhandler/dev_disk.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/winsup/cygwin/fhandler/dev_disk.cc b/winsup/cygwin/fhandler/dev_disk.cc
index 016b4c7bc..c5d72816f 100644
--- a/winsup/cygwin/fhandler/dev_disk.cc
+++ b/winsup/cygwin/fhandler/dev_disk.cc
@@ -308,7 +308,7 @@ partition_to_label_or_uuid(bool uuid, const UNICODE_STRING *drive_uname,
       && nvdb->VolumeSerialNumber.QuadPart)
     {
       /* Print without any separator as on Linux. */
-      __small_sprintf (name, "%16X", nvdb->VolumeSerialNumber.QuadPart);
+      __small_sprintf (name, "%016X", nvdb->VolumeSerialNumber.QuadPart);
       NtClose(volhdl);
       return true;
     }
-- 
2.42.1