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

[Corinna Vinschen <corinna-cygwin@cygwin.com>]

On Nov 21 19:31, Christian Franke wrote:
> Corinna Vinschen wrote:
> > Hi Christian,
> > 
> > Looks good, but I just realized that I was already wondering about the
> > sanitization and forgot to talk about it:
> > 
> > On Nov 21 12:24, Christian Franke wrote:
> > > 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
> > Apart from slash and backslash, we don't have this problem in Cygwin,
> > usually.  Even control characters are no problem.  All chars not allowed
> > in filenames are just transposed into the Unicode private use area, as
> > per strfuncs.cc, line 20ff on the way to storage, and back when reading
> > the names from storage.  This, and especially in a virtual filesystem
> > like /proc, there's no reason to avoid these characters.
> 
> Thanks for clarification.
> 
> 
> > 
> > > +         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'"');
> > If you really want to avoid chars not allowed in DOS filenames, the
> > list seems incomplete, missing '<', '>', '?', '*', '|'.
> > 
> > But as I said, there's really no reason for that.  I simply reduced the
> > above expression to
> > 
> >    return !(c == L'/' || c == L'\\' || c == L'#');
> > 
> > and created a disk label
> > 
> >    test"foo*bar?baz:"
> > 
> > It works nicely, including stuff like
> > 
> >    $ ls *\"*
> >    $ ls *\**
> > 
> > So, I can push it as is, or we just allow everything and the kitchen sink
> > as per the reduced filter expression.  What do you prefer?
> 
> The latter - patch attached.

Pushed.

Thanks a lot,
Corinna