From: Christian Franke <Christian.Franke@t-online.de>
To: cygwin-patches@cygwin.com
Subject: Re: [PATCH] Cygwin: Add /dev/disk/by-label and /dev/disk/by-uuid symlinks
Date: Tue, 21 Nov 2023 12:24:20 +0100 [thread overview]
Message-ID: <0ba1c78e-15e6-65a2-eb4d-16ac2495c356@t-online.de> (raw)
In-Reply-To: <0f8c8b7e-8a67-bc0a-24c3-91d28e2f0972@t-online.de>
[-- Attachment #1: Type: text/plain, Size: 2340 bytes --]
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.
[-- Attachment #2: 0001-Cygwin-dev-disk-Append-N-if-the-same-name-appears-mo.patch --]
[-- Type: text/plain, Size: 4665 bytes --]
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
next prev parent reply other threads:[~2023-11-21 11:24 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-17 14:39 Christian Franke
2023-11-17 16:23 ` Corinna Vinschen
2023-11-17 16:45 ` Christian Franke
2023-11-17 16:49 ` Corinna Vinschen
2023-11-17 17:53 ` Christian Franke
2023-11-17 19:40 ` Corinna Vinschen
2023-11-17 20:25 ` Christian Franke
2023-11-20 9:40 ` Corinna Vinschen
2023-11-20 9:46 ` Corinna Vinschen
2023-11-20 14:54 ` Christian Franke
2023-11-20 20:02 ` Corinna Vinschen
2023-11-21 11:24 ` Christian Franke [this message]
2023-11-21 15:24 ` Corinna Vinschen
2023-11-21 18:31 ` Christian Franke
2023-11-21 18:41 ` Corinna Vinschen
2023-11-22 9:18 ` Corinna Vinschen
2023-11-22 16:31 ` Christian Franke
2023-11-23 16:27 ` Corinna Vinschen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0ba1c78e-15e6-65a2-eb4d-16ac2495c356@t-online.de \
--to=christian.franke@t-online.de \
--cc=cygwin-patches@cygwin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).