* [PATCH] Cygwin: utils: chattr: Improve option parsing. @ 2021-05-19 15:46 Christian Franke 2021-05-19 17:47 ` Corinna Vinschen 0 siblings, 1 reply; 7+ messages in thread From: Christian Franke @ 2021-05-19 15:46 UTC (permalink / raw) To: cygwin-patches [-- Attachment #1: Type: text/plain, Size: 668 bytes --] This possibly improves the usability of chattr for some typical use cases: Command : Old : New behavior ================================================ chattr -h : help : help chattr -h FILE : help : chattr -- -h -- FILE chattr -hs FILE : help : chattr -- -h -s -- FILE chattr -sh FILE : fail : chattr -- -s -h -- FILE chattr -ar FILE : fail : chattr -- -a -r -- FILE Unrelated: there a two trivial block-copied-but-not-changed issues: $ egrep 'ACL|--r' chattr.c "Get POSIX ACL information\n" " -R, --recursive recursively list attributes of directories and their \n" Regards, Christian [-- Attachment #2: 0001-Cygwin-utils-chattr-Improve-option-parsing.patch --] [-- Type: text/plain, Size: 1370 bytes --] From 865a5a50501f3fd0cf5ed28500d3e6e45a6456de Mon Sep 17 00:00:00 2001 From: Christian Franke <christian.franke@t-online.de> Date: Wed, 19 May 2021 16:24:47 +0200 Subject: [PATCH] Cygwin: utils: chattr: Improve option parsing. Interpret '-h' as '--help' only if last argument. Allow multiple characters in first '-mode' argument. Signed-off-by: Christian Franke <christian.franke@t-online.de> --- winsup/utils/chattr.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/winsup/utils/chattr.c b/winsup/utils/chattr.c index 98f693aab..f6ce340b4 100644 --- a/winsup/utils/chattr.c +++ b/winsup/utils/chattr.c @@ -271,7 +271,7 @@ int main (int argc, char **argv) { int c, ret = 0; - int lastoptind = 0; + int lastoptind = 1; char *opt; opterr = 0; @@ -295,15 +295,16 @@ main (int argc, char **argv) print_version (); return 0; break; + case 'h': + /* Print help if -h is last argument or --help is used, + otherwise interpret -h as 'remove hidden attribute'. */ + if (optind >= argc || (optind > lastoptind && argv[optind-1][1] == '-')) + usage (stdout); + /*FALLTHRU*/ default: if (optind > lastoptind) - { - --optind; - goto next; - } - /*FALLTHRU*/ - case 'h': - usage (c == 'h' ? stdout : stderr); + --optind; + goto next; } } next: -- 2.31.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Cygwin: utils: chattr: Improve option parsing. 2021-05-19 15:46 [PATCH] Cygwin: utils: chattr: Improve option parsing Christian Franke @ 2021-05-19 17:47 ` Corinna Vinschen 2021-05-20 10:01 ` Christian Franke 0 siblings, 1 reply; 7+ messages in thread From: Corinna Vinschen @ 2021-05-19 17:47 UTC (permalink / raw) To: cygwin-patches Hi Christian, On May 19 17:46, Christian Franke wrote: > This possibly improves the usability of chattr for some typical use cases: > > Command : Old : New behavior > ================================================ > chattr -h : help : help > chattr -h FILE : help : chattr -- -h -- FILE > chattr -hs FILE : help : chattr -- -h -s -- FILE > chattr -sh FILE : fail : chattr -- -s -h -- FILE > chattr -ar FILE : fail : chattr -- -a -r -- FILE > > Unrelated: there a two trivial block-copied-but-not-changed issues: > > $ egrep 'ACL|--r' chattr.c > "Get POSIX ACL information\n" > " -R, --recursive recursively list attributes of directories and > their \n" Oops. Please patch while you're at it... > > Regards, > Christian > > From 865a5a50501f3fd0cf5ed28500d3e6e45a6456de Mon Sep 17 00:00:00 2001 > From: Christian Franke <christian.franke@t-online.de> > Date: Wed, 19 May 2021 16:24:47 +0200 > Subject: [PATCH] Cygwin: utils: chattr: Improve option parsing. > > Interpret '-h' as '--help' only if last argument. Who was the idiot using -h for help *and* the hidden flag? *blush* I'd vote for --help to be changed to -H for the single character option. The help output is very unlikely to be used in scripts, so that shouldn't be a backward compat problem. Would you mind to change the patch accordingly? Thanks, Corinna ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Cygwin: utils: chattr: Improve option parsing. 2021-05-19 17:47 ` Corinna Vinschen @ 2021-05-20 10:01 ` Christian Franke 2021-05-20 17:57 ` Corinna Vinschen 2021-05-20 20:40 ` Corinna Vinschen 0 siblings, 2 replies; 7+ messages in thread From: Christian Franke @ 2021-05-20 10:01 UTC (permalink / raw) To: cygwin-patches [-- Attachment #1: Type: text/plain, Size: 1039 bytes --] Corinna Vinschen wrote: > Hi Christian, > > On May 19 17:46, Christian Franke wrote: >> ... >> $ egrep 'ACL|--r' chattr.c >> "Get POSIX ACL information\n" >> " -R, --recursive recursively list attributes of directories and >> their \n" > Oops. Please patch while you're at it... > ... >> From 865a5a50501f3fd0cf5ed28500d3e6e45a6456de Mon Sep 17 00:00:00 2001 >> From: Christian Franke<...> >> Date: Wed, 19 May 2021 16:24:47 +0200 >> Subject: [PATCH] Cygwin: utils: chattr: Improve option parsing. >> >> Interpret '-h' as '--help' only if last argument. > Who was the idiot using -h for help *and* the hidden flag? *blush* > > I'd vote for --help to be changed to -H for the single character > option. The help output is very unlikely to be used in scripts, > so that shouldn't be a backward compat problem. New patch attached. Note that there is now the possibly unexpected (& hidden) behavior that 'chattr -h' without file argument clears the hidden attribute of cwd. Regards, Christian [-- Attachment #2: 0001-Cygwin-utils-chattr-Improve-option-parsing-fix-some-.patch --] [-- Type: text/plain, Size: 5050 bytes --] From b838125f797c123d4a6d2f0ef30cccee8face50c Mon Sep 17 00:00:00 2001 From: Christian Franke <christian.franke@t-online.de> Date: Thu, 20 May 2021 11:05:29 +0200 Subject: [PATCH] Cygwin: utils: chattr: Improve option parsing, fix some messages. Allow multiple characters also in first '-mode' argument. Use '-H' instead of '-h' for '--help' to fix ambiguity with hidden attribute. Fix help and usage texts and documentation. Signed-off-by: Christian Franke <christian.franke@t-online.de> --- winsup/doc/utils.xml | 10 +++++----- winsup/utils/chattr.c | 32 ++++++++++++++------------------ 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/winsup/doc/utils.xml b/winsup/doc/utils.xml index 22bd86904..69611b954 100644 --- a/winsup/doc/utils.xml +++ b/winsup/doc/utils.xml @@ -27,18 +27,18 @@ <refsynopsisdiv> <screen> -chattr [-RVfhv] [+-=mode]... [file]... +chattr [-RVfHv] [+-=mode]... [file]... </screen> </refsynopsisdiv> <refsect1 id="chattr-options"> <title>Options</title> <screen> - -R, --recursive recursively list attributes of directories and their - contents + -R, --recursive recursively apply the changes to directories and + their contents -V, --verbose Be verbose during operation -f, --force suppress error messages - -h, --help this help text + -H, --help this help text -v, --version display the program version </screen> </refsect1> @@ -60,7 +60,7 @@ chattr [-RVfhv] [+-=mode]... [file]... <para>Supported attributes:</para> <screen> - 'r', 'Readonly': file is read-only + 'r', 'Readonly': file is read-only 'h', 'Hidden': file or directory is hidden 's', 'System': file or directory that the operating system uses 'a', 'Archive': file or directory has the archive marker set diff --git a/winsup/utils/chattr.c b/winsup/utils/chattr.c index 98f693aab..c7dc649c2 100644 --- a/winsup/utils/chattr.c +++ b/winsup/utils/chattr.c @@ -28,12 +28,12 @@ struct option longopts[] = { { "recursive", no_argument, NULL, 'R' }, { "verbose", no_argument, NULL, 'V' }, { "force", no_argument, NULL, 'f' }, - { "help", no_argument, NULL, 'h' }, + { "help", no_argument, NULL, 'H' }, { "version", no_argument, NULL, 'v' }, { NULL, no_argument, NULL, 0} }; -const char *opts = "+RVfhv"; +const char *opts = "+RVfHv"; struct { @@ -211,7 +211,7 @@ static void print_version () { printf ("%s (cygwin) %d.%d.%d\n" - "Get POSIX ACL information\n" + "Change file attributes\n" "Copyright (C) 2018 - %s Cygwin Authors\n" "This is free software; see the source for copying conditions. " "There is NO\n" @@ -227,7 +227,7 @@ print_version () static void __attribute__ ((__noreturn__)) usage (FILE *stream) { - fprintf (stream, "Usage: %s [-RVfhv] [+-=mode]... [file]...\n", + fprintf (stream, "Usage: %s [-RVfHv] [+-=mode]... [file]...\n", program_invocation_short_name); if (stream == stderr) fprintf (stream, "Try '%s --help' for more information\n", @@ -236,11 +236,11 @@ usage (FILE *stream) fprintf (stream, "\n" "Change file attributes\n" "\n" - " -R, --recursive recursively list attributes of directories and their \n" + " -R, --recursive recursively apply the changes to directories and their\n" " contents\n" " -V, --verbose Be verbose during operation\n" " -f, --force suppress error messages\n" - " -h, --help this help text\n" + " -H, --help this help text\n" " -v, --version display the program version\n" "\n" "The format of 'mode' is {+-=}[acCehnrsSt]\n" @@ -251,7 +251,7 @@ usage (FILE *stream) "\n" "Supported attributes:\n" "\n" - " 'r', 'Readonly': file is read-only\n" + " 'r', 'Readonly': file is read-only\n" " 'h', 'Hidden': file or directory is hidden\n" " 's', 'System': file or directory that the operating system uses\n" " 'a', 'Archive': file or directory has the archive marker set\n" @@ -271,7 +271,7 @@ int main (int argc, char **argv) { int c, ret = 0; - int lastoptind = 0; + int lastoptind = 1; char *opt; opterr = 0; @@ -281,15 +281,15 @@ main (int argc, char **argv) { case 'R': Ropt = 1; - lastoptind = optind; break; case 'V': Vopt = 1; - lastoptind = optind; break; case 'f': fopt = 1; - lastoptind = optind; + break; + case 'H': + usage (stdout); break; case 'v': print_version (); @@ -297,14 +297,10 @@ main (int argc, char **argv) break; default: if (optind > lastoptind) - { - --optind; - goto next; - } - /*FALLTHRU*/ - case 'h': - usage (c == 'h' ? stdout : stderr); + --optind; + goto next; } + lastoptind = optind; } next: while (optind < argc) -- 2.31.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Cygwin: utils: chattr: Improve option parsing. 2021-05-20 10:01 ` Christian Franke @ 2021-05-20 17:57 ` Corinna Vinschen 2021-05-20 19:48 ` Christian Franke 2021-05-20 20:40 ` Corinna Vinschen 1 sibling, 1 reply; 7+ messages in thread From: Corinna Vinschen @ 2021-05-20 17:57 UTC (permalink / raw) To: cygwin-patches Hi Christian, On May 20 12:01, Christian Franke wrote: > Corinna Vinschen wrote: > > On May 19 17:46, Christian Franke wrote: > > > ... > > > $ egrep 'ACL|--r' chattr.c > > > "Get POSIX ACL information\n" > > > " -R, --recursive recursively list attributes of directories and > > > their \n" > > Oops. Please patch while you're at it... > > ... > > > From 865a5a50501f3fd0cf5ed28500d3e6e45a6456de Mon Sep 17 00:00:00 2001 > > > From: Christian Franke<...> > > > Date: Wed, 19 May 2021 16:24:47 +0200 > > > Subject: [PATCH] Cygwin: utils: chattr: Improve option parsing. > > > > > > Interpret '-h' as '--help' only if last argument. > > Who was the idiot using -h for help *and* the hidden flag? *blush* > > > > I'd vote for --help to be changed to -H for the single character > > option. The help output is very unlikely to be used in scripts, > > so that shouldn't be a backward compat problem. > > New patch attached. > > Note that there is now the possibly unexpected (& hidden) behavior that > 'chattr -h' without file argument clears the hidden attribute of cwd. Uhm... why? Isn't that easily avoidable? Thanks, Corinna ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Cygwin: utils: chattr: Improve option parsing. 2021-05-20 17:57 ` Corinna Vinschen @ 2021-05-20 19:48 ` Christian Franke 2021-05-20 20:19 ` Corinna Vinschen 0 siblings, 1 reply; 7+ messages in thread From: Christian Franke @ 2021-05-20 19:48 UTC (permalink / raw) To: cygwin-patches Hi Corinna, Corinna Vinschen wrote: > Hi Christian, > > On May 20 12:01, Christian Franke wrote: >> Corinna Vinschen wrote: >>> On May 19 17:46, Christian Franke wrote: >>>> ... >>>> $ egrep 'ACL|--r' chattr.c >>>> "Get POSIX ACL information\n" >>>> " -R, --recursive recursively list attributes of directories and >>>> their \n" >>> Oops. Please patch while you're at it... >>> ... >>>> From 865a5a50501f3fd0cf5ed28500d3e6e45a6456de Mon Sep 17 00:00:00 2001 >>>> From: Christian Franke<...> >>>> Date: Wed, 19 May 2021 16:24:47 +0200 >>>> Subject: [PATCH] Cygwin: utils: chattr: Improve option parsing. >>>> >>>> Interpret '-h' as '--help' only if last argument. >>> Who was the idiot using -h for help *and* the hidden flag? *blush* >>> >>> I'd vote for --help to be changed to -H for the single character >>> option. The help output is very unlikely to be used in scripts, >>> so that shouldn't be a backward compat problem. >> New patch attached. >> >> Note that there is now the possibly unexpected (& hidden) behavior that >> 'chattr -h' without file argument clears the hidden attribute of cwd. > Uhm... why? Because chattr uses '.' as default if no FILE argument is specified. The same applies to all other '+-=MODE' arguments. The patch does not change this behavior but of course enables it for '-h'. > Isn't that easily avoidable? Yes: make FILE argument mandatory - but this would break backward compatibility. Another behavior (possibly inherited from lsattr) is also not very useful: 'chattr +-=MODE DIRECTORY' also changes the attributes of all elements in the directory (not recursively). It is not possible to solely change the attributes of a directory except if it is the current directory and no FILE argument is passed. Fixing this would again break backward compatibility. Regards, Christian ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Cygwin: utils: chattr: Improve option parsing. 2021-05-20 19:48 ` Christian Franke @ 2021-05-20 20:19 ` Corinna Vinschen 0 siblings, 0 replies; 7+ messages in thread From: Corinna Vinschen @ 2021-05-20 20:19 UTC (permalink / raw) To: cygwin-patches On May 20 21:48, Christian Franke wrote: > Hi Corinna, > > Corinna Vinschen wrote: > > Hi Christian, > > > > On May 20 12:01, Christian Franke wrote: > > > Corinna Vinschen wrote: > > > > On May 19 17:46, Christian Franke wrote: > > > > > ... > > > > > $ egrep 'ACL|--r' chattr.c > > > > > "Get POSIX ACL information\n" > > > > > " -R, --recursive recursively list attributes of directories and > > > > > their \n" > > > > Oops. Please patch while you're at it... > > > > ... > > > > > From 865a5a50501f3fd0cf5ed28500d3e6e45a6456de Mon Sep 17 00:00:00 2001 > > > > > From: Christian Franke<...> > > > > > Date: Wed, 19 May 2021 16:24:47 +0200 > > > > > Subject: [PATCH] Cygwin: utils: chattr: Improve option parsing. > > > > > > > > > > Interpret '-h' as '--help' only if last argument. > > > > Who was the idiot using -h for help *and* the hidden flag? *blush* > > > > > > > > I'd vote for --help to be changed to -H for the single character > > > > option. The help output is very unlikely to be used in scripts, > > > > so that shouldn't be a backward compat problem. > > > New patch attached. > > > > > > Note that there is now the possibly unexpected (& hidden) behavior that > > > 'chattr -h' without file argument clears the hidden attribute of cwd. > > Uhm... why? > > Because chattr uses '.' as default if no FILE argument is specified. The > same applies to all other '+-=MODE' arguments. The patch does not change > this behavior but of course enables it for '-h'. Oh, right. I can't reproduce this behaviour with the most recent chattr from e2fsprogs in F33. Sorry, I'm not sure anymore why I did that. But given the entire behaviour is rather unexpected, and it's not even documented, maybe we should drop it, too? Not sure if backward compat counts in this case... > Another behavior (possibly inherited from lsattr) is also not very useful: > 'chattr +-=MODE DIRECTORY' also changes the attributes of all elements in > the directory (not recursively). It is not possible to solely change the > attributes of a directory except if it is the current directory and no FILE > argument is passed. Fixing this would again break backward compatibility. Ouch, no! That's a bug. The expression in line 354 should have been if (S_ISDIR (st.st_mode) && Ropt && chattr_dir (argv[optind])) ret = 1; the Ropt check is just missing. Corinna ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Cygwin: utils: chattr: Improve option parsing. 2021-05-20 10:01 ` Christian Franke 2021-05-20 17:57 ` Corinna Vinschen @ 2021-05-20 20:40 ` Corinna Vinschen 1 sibling, 0 replies; 7+ messages in thread From: Corinna Vinschen @ 2021-05-20 20:40 UTC (permalink / raw) To: cygwin-patches On May 20 12:01, Christian Franke wrote: > Corinna Vinschen wrote: > > Hi Christian, > > > > On May 19 17:46, Christian Franke wrote: > > > ... > > > $ egrep 'ACL|--r' chattr.c > > > "Get POSIX ACL information\n" > > > " -R, --recursive recursively list attributes of directories and > > > their \n" > > Oops. Please patch while you're at it... > > ... > > > From 865a5a50501f3fd0cf5ed28500d3e6e45a6456de Mon Sep 17 00:00:00 2001 > > > From: Christian Franke<...> > > > Date: Wed, 19 May 2021 16:24:47 +0200 > > > Subject: [PATCH] Cygwin: utils: chattr: Improve option parsing. > > > > > > Interpret '-h' as '--help' only if last argument. > > Who was the idiot using -h for help *and* the hidden flag? *blush* > > > > I'd vote for --help to be changed to -H for the single character > > option. The help output is very unlikely to be used in scripts, > > so that shouldn't be a backward compat problem. > > New patch attached. > > Note that there is now the possibly unexpected (& hidden) behavior that > 'chattr -h' without file argument clears the hidden attribute of cwd. > > Regards, > Christian > > From b838125f797c123d4a6d2f0ef30cccee8face50c Mon Sep 17 00:00:00 2001 > From: Christian Franke <christian.franke@t-online.de> > Date: Thu, 20 May 2021 11:05:29 +0200 > Subject: [PATCH] Cygwin: utils: chattr: Improve option parsing, fix some > messages. > > Allow multiple characters also in first '-mode' argument. > Use '-H' instead of '-h' for '--help' to fix ambiguity with > hidden attribute. Fix help and usage texts and documentation. > > Signed-off-by: Christian Franke <christian.franke@t-online.de> > --- > winsup/doc/utils.xml | 10 +++++----- > winsup/utils/chattr.c | 32 ++++++++++++++------------------ > 2 files changed, 19 insertions(+), 23 deletions(-) Pushed. Thanks, Corinna ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-05-20 20:40 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-19 15:46 [PATCH] Cygwin: utils: chattr: Improve option parsing Christian Franke 2021-05-19 17:47 ` Corinna Vinschen 2021-05-20 10:01 ` Christian Franke 2021-05-20 17:57 ` Corinna Vinschen 2021-05-20 19:48 ` Christian Franke 2021-05-20 20:19 ` Corinna Vinschen 2021-05-20 20:40 ` Corinna Vinschen
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).