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

Christian Franke Christian.Franke@t-online.de
Tue Nov 21 11:24:20 GMT 2023


Christian Franke wrote:
> 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}.
>

Attached.

-------------- next part --------------
From 7dafb85210ef77ea8798f22160f7782c394ef5c3 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Tue, 21 Nov 2023 12:17:14 +0100
Subject: [PATCH] Cygwin: /dev/disk: Append '#N' if the same name appears more
 than once

No longer drop ranges of identical link names.  Append '#0, #1, ...'
to each name instead.  No longer ignore null volume serial numbers.

Signed-off-by: Christian Franke <christian.franke@t-online.de>
---
 winsup/cygwin/fhandler/dev_disk.cc | 53 +++++++++++++++++++-----------
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/winsup/cygwin/fhandler/dev_disk.cc b/winsup/cygwin/fhandler/dev_disk.cc
index c5d72816f..d12ac52fa 100644
--- a/winsup/cygwin/fhandler/dev_disk.cc
+++ b/winsup/cygwin/fhandler/dev_disk.cc
@@ -64,10 +64,12 @@ sanitize_label_string (WCHAR *s)
   /* Linux does not skip leading spaces. */
   return sanitize_string (s, L'\0', L' ', L'_', [] (WCHAR c) -> bool
     {
-      /* Labels may contain characters not allowed in filenames.
-	 Linux replaces spaces with \x20 which is not an option here. */
+      /* Labels may contain characters not allowed in filenames.  Also
+         replace '#' to avoid that duplicate markers introduce new
+	 duplicates.  Linux replaces spaces with \x20 which is not an
+	 option here. */
       return !((0 <= c && c <= L' ') || c == L':' || c == L'/' || c == L'\\'
-	      || c == L'"');
+	      || c == L'#' || c == L'"');
     }
   );
 }
@@ -304,8 +306,7 @@ partition_to_label_or_uuid(bool uuid, const UNICODE_STRING *drive_uname,
   const NTFS_VOLUME_DATA_BUFFER *nvdb =
     reinterpret_cast<const NTFS_VOLUME_DATA_BUFFER *>(ioctl_buf);
   if (uuid && DeviceIoControl (volhdl, FSCTL_GET_NTFS_VOLUME_DATA, nullptr, 0,
-			       ioctl_buf, NT_MAX_PATH, &bytes_read, nullptr)
-      && nvdb->VolumeSerialNumber.QuadPart)
+			       ioctl_buf, NT_MAX_PATH, &bytes_read, nullptr))
     {
       /* Print without any separator as on Linux. */
       __small_sprintf (name, "%016X", nvdb->VolumeSerialNumber.QuadPart);
@@ -327,13 +328,9 @@ partition_to_label_or_uuid(bool uuid, const UNICODE_STRING *drive_uname,
   FILE_FS_VOLUME_INFORMATION *ffvi =
     reinterpret_cast<FILE_FS_VOLUME_INFORMATION *>(ioctl_buf);
   if (uuid)
-    {
-      if (!ffvi->VolumeSerialNumber)
-	return false;
-      /* Print with separator as on Linux. */
-      __small_sprintf (name, "%04x-%04x", ffvi->VolumeSerialNumber >> 16,
-		       ffvi->VolumeSerialNumber & 0xffff);
-    }
+    /* Print with separator as on Linux. */
+    __small_sprintf (name, "%04x-%04x", ffvi->VolumeSerialNumber >> 16,
+		     ffvi->VolumeSerialNumber & 0xffff);
   else
     {
       /* Label is not null terminated. */
@@ -361,6 +358,20 @@ by_id_compare_name (const void *a, const void *b)
   return strcmp (ap->name, bp->name);
 }
 
+static int
+by_id_compare_name_drive_part (const void *a, const void *b)
+{
+  const by_id_entry *ap = reinterpret_cast<const by_id_entry *>(a);
+  const by_id_entry *bp = reinterpret_cast<const by_id_entry *>(b);
+  int cmp = strcmp (ap->name, bp->name);
+  if (cmp)
+    return cmp;
+  cmp = ap->drive - bp->drive;
+  if (cmp)
+    return cmp;
+  return ap->part - bp->part;
+}
+
 static by_id_entry *
 by_id_realloc (by_id_entry *p, size_t n)
 {
@@ -610,8 +621,9 @@ 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. */
-  qsort (table, table_size, sizeof (*table), by_id_compare_name);
+  /* Sort by {name, drive, part} to ensure stable sort order. */
+  qsort (table, table_size, sizeof (*table), by_id_compare_name_drive_part);
+  /* Mark duplicate names. */
   for (unsigned i = 0; i < table_size; i++)
     {
       unsigned j = i + 1;
@@ -619,12 +631,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
+	 introduces new duplicates because '#' never occurs in the
+	 original names. */
+      debug_printf ("mark duplicates %u-%u of '%s'", i, j - 1, table[i].name);
+      size_t len = strlen (table[i].name);
+      for (unsigned k = i; k < j; k++)
+	__small_sprintf (table[k].name + len, "#%u", k - i);
     }
 
   debug_printf ("table_size: %d", table_size);
-- 
2.42.1



More information about the Cygwin-patches mailing list