Re: [PATCH] Cygwin: utils: chattr: Improve option parsing.

[Christian Franke <Christian.Franke@t-online.de>]

[-- 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