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.

From 7dafb85210ef77ea8798f22160f7782c394ef5c3 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.fra...@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.fra...@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

Reply via email to