On 11/27/18 10:04 PM, Paul Eggert wrote:
> I'd rather see a function than a big macro like that. Can we do it as a 
> function instead?
> 
> Is the idea to deprecate and/or stop using parse_long_options, since it 
> doesn't work the way the Gnu Coding Standards says it should? If so, 
> maybe it would be simpler to change parse_long_options instead. This 
> could involve changing the API for parse_long_options.

Thank you very much both for your comments.

Indeed, parse_long_options is not good enough for our purposes
in the following regards:

* it only runs if (argc == 2), i.e., it doesn't handle e.g.
    $ yes --help me

* it stops at the first non-option argument, i.e., it doesn't allow
to handle the GNU standard options after an argument, e.g.
    $ dd if=... --help
(The above is especially nice if one already typed a quite long dd command,
and needs to know one another option, so one could just append --help, and
then re-edit the command replacing --help by the then-known other option.)

* it does not allow to differentiate between the need to scan all
arguments vs. stopping at the first non-option argument;   this is
important for programs launching other programs.  E.g. nohup should
not gobble the --help option after COMMAND:
    $ nohup COMMAND --help

* if a command does not allow other options, then another getopt_long call
is needed.

The attached are quite raw attempts to address this - yes, as a function
instead of a macro. ;-)

The idea is to have a function which allows only the --help or --version
option, and complaining about any other option (well, programs allowing
other options besides the GNU standard options will have their own
getopt_long loop anyway).  And it allows by the SCAN_ALL parameter to
stop or not stop at the 1st non-option argument.

* [PATCH] long-options: add parse_gnu_standard_options_only
  gnulib patch!

* [PATCH] all: detect --help and --version more consistently [FIXME]
  FIXME: NEWS, syntax-check, tests.

Have a nice day,
Berny
>From a882a0bdf4b2d19b25f42bb2d4194b526dde8589 Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <m...@bernhard-voelker.de>
Date: Thu, 29 Nov 2018 09:06:26 +0100
Subject: [PATCH] long-options: add parse_gnu_standard_options_only

* lib/long-options.c (parse_long_options): Use EXIT_SUCCESS instead
of 0.
(parse_gnu_standard_options_only): Add function to
process the GNU default options --help and --version (see also [1]),
and fail for any other unknown long or short option.
* lib/long-options.h (parse_gnu_standard_options_only): Declare it.
[1]
https://gnu.org/prep/standards/html_node/Command_002dLine-Interfaces.html
---
 ChangeLog            | 12 +++++++++
 lib/long-options.c   | 58 +++++++++++++++++++++++++++++++++++++++++++-
 lib/long-options.h   | 16 ++++++++++++
 modules/long-options |  1 +
 4 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index af7a14fa9..3b25962b6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2018-11-29  Bernhard Voelker  <m...@bernhard-voelker.de>
+
+	long-options: add parse_gnu_standard_options_only
+	* lib/long-options.c (parse_long_options): Use EXIT_SUCCESS instead
+	of 0.
+	(parse_gnu_standard_options_only): Add function to
+	process the GNU default options --help and --version (see also [1]),
+	and fail for any other unknown long or short option.
+	* lib/long-options.h (parse_gnu_standard_options_only): Declare it.
+	[1]
+	https://gnu.org/prep/standards/html_node/Command_002dLine-Interfaces.html
+
 2018-11-29  Akim Demaille  <a...@lrde.epita.fr>
 
 	bitset: rename ebitset/expandable.* as tbitset/table.*
diff --git a/lib/long-options.c b/lib/long-options.c
index eef1f2f35..b3f1df404 100644
--- a/lib/long-options.c
+++ b/lib/long-options.c
@@ -71,7 +71,7 @@ parse_long_options (int argc,
             va_list authors;
             va_start (authors, usage_func);
             version_etc_va (stdout, command_name, package, version, authors);
-            exit (0);
+            exit (EXIT_SUCCESS);
           }
 
         default:
@@ -87,3 +87,59 @@ parse_long_options (int argc,
      the probably-new parameters when/if getopt is called later.  */
   optind = 0;
 }
+
+/* Process the GNU default long options --help and --version (see also
+   https://gnu.org/prep/standards/html_node/Command_002dLine-Interfaces.html),
+   and fail for any other unknown long or short option.
+   Use with SCAN_ALL=true to scan until "--", or with SCAN_ALL=false to stop
+   at the first non-option argument (or "--", whichever comes first).  */
+
+void
+parse_gnu_standard_options_only (int argc,
+                                 char **argv,
+                                 const char *command_name,
+                                 const char *package,
+                                 const char *version,
+                                 bool scan_all,
+                                 void (*usage_func) (int),
+                                 /* const char *author1, ...*/ ...)
+{
+  int c;
+  int saved_opterr;
+
+  saved_opterr = opterr;
+
+  /* Print an error message for unrecognized options.  */
+  opterr = 1;
+
+  const char *optstring = scan_all ? "" : "+";
+
+  if ((c = getopt_long (argc, argv, optstring, long_options, NULL)) != -1)
+    {
+      switch (c)
+        {
+        case 'h':
+          (*usage_func) (EXIT_SUCCESS);
+          break;
+
+        case 'v':
+          {
+            va_list authors;
+            va_start (authors, usage_func);
+            version_etc_va (stdout, command_name, package, version, authors);
+            exit (EXIT_SUCCESS);
+          }
+
+        default:
+          (*usage_func) (EXIT_FAILURE);
+          break;
+        }
+    }
+
+  /* Restore previous value.  */
+  opterr = saved_opterr;
+
+  /* Reset this to zero so that getopt internals get initialized from
+     the probably-new parameters when/if getopt is called later.  */
+  optind = 0;
+}
diff --git a/lib/long-options.h b/lib/long-options.h
index 109a20375..44d6aa6ab 100644
--- a/lib/long-options.h
+++ b/lib/long-options.h
@@ -17,6 +17,11 @@
 
 /* Written by Jim Meyering.  */
 
+#ifndef LONG_OPTIONS_H_
+# define LONG_OPTIONS_H_ 1
+
+# include <stdbool.h>
+
 void parse_long_options (int _argc,
                          char **_argv,
                          const char *_command_name,
@@ -24,3 +29,14 @@ void parse_long_options (int _argc,
                          const char *_version,
                          void (*_usage) (int),
                          /* const char *author1, ...*/ ...);
+
+void parse_gnu_standard_options_only (int argc,
+                                      char **argv,
+                                      const char *command_name,
+                                      const char *package,
+                                      const char *version,
+                                      bool scan_all,
+                                      void (*usage_func) (int),
+                                      /* const char *author1, ...*/ ...);
+
+#endif /* LONG_OPTIONS_H_ */
diff --git a/modules/long-options b/modules/long-options
index f1106c3cc..9bec13e75 100644
--- a/modules/long-options
+++ b/modules/long-options
@@ -7,6 +7,7 @@ lib/long-options.c
 
 Depends-on:
 getopt-gnu
+stdbool
 stdlib
 version-etc
 
-- 
2.19.1

>From f7522adbd78d86c3b81afb56cd9025a5645de0d6 Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <m...@bernhard-voelker.de>
Date: Mon, 26 Nov 2018 09:05:37 +0100
Subject: [PATCH] all: detect --help and --version more consistently [FIXME]

FIXME: NEWS, syntax-check, tests.
---
 src/cksum.c    | 11 ++---------
 src/dd.c       | 12 ++----------
 src/hostid.c   | 11 ++---------
 src/hostname.c | 11 ++---------
 src/link.c     | 11 ++---------
 src/logname.c  | 11 ++---------
 src/nohup.c    | 11 ++---------
 src/sleep.c    | 11 ++---------
 src/tsort.c    | 11 ++---------
 src/unlink.c   | 11 ++---------
 src/uptime.c   | 11 ++---------
 src/users.c    | 11 ++---------
 src/whoami.c   | 11 ++---------
 src/yes.c      | 11 ++---------
 14 files changed, 28 insertions(+), 127 deletions(-)

diff --git a/src/cksum.c b/src/cksum.c
index 372d6b601..8da1ec608 100644
--- a/src/cksum.c
+++ b/src/cksum.c
@@ -107,11 +107,6 @@ main (void)
 # include "die.h"
 # include "error.h"
 
-static struct option const long_options[] =
-{
-  {NULL, 0, NULL, 0}
-};
-
 /* Number of bytes to read at once.  */
 # define BUFLEN (1 << 16)
 
@@ -294,10 +289,8 @@ main (int argc, char **argv)
      so that processes running in parallel do not intersperse their output.  */
   setvbuf (stdout, NULL, _IOLBF, 0);
 
-  parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE, Version,
-                      usage, AUTHORS, (char const *) NULL);
-  if (getopt_long (argc, argv, "", long_options, NULL) != -1)
-    usage (EXIT_FAILURE);
+  parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE, Version,
+                                   true, usage, AUTHORS, (char const *) NULL);
 
   have_read_stdin = false;
 
diff --git a/src/dd.c b/src/dd.c
index 6aeef520a..2b4f23145 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -46,11 +46,6 @@
   proper_name ("David MacKenzie"), \
   proper_name ("Stuart Kemp")
 
-static struct option const long_options[] =
-{
-  {NULL, 0, NULL, 0}
-};
-
 /* Use SA_NOCLDSTOP as a proxy for whether the sigaction machinery is
    present.  */
 #ifndef SA_NOCLDSTOP
@@ -2396,13 +2391,10 @@ main (int argc, char **argv)
 
   page_size = getpagesize ();
 
-  parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE, Version,
-                      usage, AUTHORS, (char const *) NULL);
+  parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE, Version,
+                                   true, usage, AUTHORS, (char const *) NULL);
   close_stdout_required = false;
 
-  if (getopt_long (argc, argv, "", long_options, NULL) != -1)
-    usage (EXIT_FAILURE);
-
   /* Initialize translation table to identity translation. */
   for (i = 0; i < 256; i++)
     trans_table[i] = i;
diff --git a/src/hostid.c b/src/hostid.c
index c45328aea..4d99b9466 100644
--- a/src/hostid.c
+++ b/src/hostid.c
@@ -32,11 +32,6 @@
 
 #define AUTHORS proper_name ("Jim Meyering")
 
-static struct option const long_options[] =
-{
-  {NULL, 0, NULL, 0}
-};
-
 void
 usage (int status)
 {
@@ -69,10 +64,8 @@ main (int argc, char **argv)
 
   atexit (close_stdout);
 
-  parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
-                      usage, AUTHORS, (char const *) NULL);
-  if (getopt_long (argc, argv, "", long_options, NULL) != -1)
-    usage (EXIT_FAILURE);
+  parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
+                                   true, usage, AUTHORS, (char const *) NULL);
 
   if (optind < argc)
     {
diff --git a/src/hostname.c b/src/hostname.c
index 292c148ec..6e68959da 100644
--- a/src/hostname.c
+++ b/src/hostname.c
@@ -33,11 +33,6 @@
 
 #define AUTHORS proper_name ("Jim Meyering")
 
-static struct option const long_options[] =
-{
-  {NULL, 0, NULL, 0}
-};
-
 #if !defined HAVE_SETHOSTNAME && defined HAVE_SYSINFO && \
      defined HAVE_SYS_SYSTEMINFO_H
 # include <sys/systeminfo.h>
@@ -86,10 +81,8 @@ main (int argc, char **argv)
 
   atexit (close_stdout);
 
-  parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
-                      usage, AUTHORS, (char const *) NULL);
-  if (getopt_long (argc, argv, "", long_options, NULL) != -1)
-    usage (EXIT_FAILURE);
+  parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
+                                   true, usage, AUTHORS, (char const *) NULL);
 
   if (argc == optind + 1)
     {
diff --git a/src/link.c b/src/link.c
index 203a11788..823aa912d 100644
--- a/src/link.c
+++ b/src/link.c
@@ -36,11 +36,6 @@
 
 #define AUTHORS proper_name ("Michael Stone")
 
-static struct option const long_options[] =
-{
-  {NULL, 0, NULL, 0}
-};
-
 void
 usage (int status)
 {
@@ -72,10 +67,8 @@ main (int argc, char **argv)
 
   atexit (close_stdout);
 
-  parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
-                      usage, AUTHORS, (char const *) NULL);
-  if (getopt_long (argc, argv, "", long_options, NULL) != -1)
-    usage (EXIT_FAILURE);
+  parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
+                                   true, usage, AUTHORS, (char const *) NULL);
 
   if (argc < optind + 2)
     {
diff --git a/src/logname.c b/src/logname.c
index 171fe483f..0706b4dd2 100644
--- a/src/logname.c
+++ b/src/logname.c
@@ -30,11 +30,6 @@
 
 #define AUTHORS proper_name ("FIXME: unknown")
 
-static struct option const long_options[] =
-{
-  {NULL, 0, NULL, 0}
-};
-
 void
 usage (int status)
 {
@@ -67,10 +62,8 @@ main (int argc, char **argv)
 
   atexit (close_stdout);
 
-  parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
-                      usage, AUTHORS, (char const *) NULL);
-  if (getopt_long (argc, argv, "", long_options, NULL) != -1)
-    usage (EXIT_FAILURE);
+  parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
+                                   true, usage, AUTHORS, (char const *) NULL);
 
   if (optind < argc)
     {
diff --git a/src/nohup.c b/src/nohup.c
index 4bc7a4a48..8ea0bae32 100644
--- a/src/nohup.c
+++ b/src/nohup.c
@@ -34,11 +34,6 @@
 
 #define AUTHORS proper_name ("Jim Meyering")
 
-static struct option const long_options[] =
-{
-  {NULL, 0, NULL, 0}
-};
-
 /* Exit statuses.  */
 enum
   {
@@ -104,10 +99,8 @@ main (int argc, char **argv)
   initialize_exit_failure (exit_internal_failure);
   atexit (close_stdout);
 
-  parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
-                      usage, AUTHORS, (char const *) NULL);
-  if (getopt_long (argc, argv, "+", long_options, NULL) != -1)
-    usage (exit_internal_failure);
+  parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
+                                   false, usage, AUTHORS, (char const *) NULL);
 
   if (argc <= optind)
     {
diff --git a/src/sleep.c b/src/sleep.c
index 162053d7c..f4323744d 100644
--- a/src/sleep.c
+++ b/src/sleep.c
@@ -35,11 +35,6 @@
   proper_name ("Jim Meyering"), \
   proper_name ("Paul Eggert")
 
-static struct option const long_options[] =
-{
-  {NULL, 0, NULL, 0}
-};
-
 void
 usage (int status)
 {
@@ -114,10 +109,8 @@ main (int argc, char **argv)
 
   atexit (close_stdout);
 
-  parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
-                      usage, AUTHORS, (char const *) NULL);
-  if (getopt_long (argc, argv, "", long_options, NULL) != -1)
-    usage (EXIT_FAILURE);
+  parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
+                                   true, usage, AUTHORS, (char const *) NULL);
 
   if (argc == 1)
     {
diff --git a/src/tsort.c b/src/tsort.c
index 0125c5c50..50d9100e6 100644
--- a/src/tsort.c
+++ b/src/tsort.c
@@ -40,11 +40,6 @@
 
 #define AUTHORS proper_name ("Mark Kettenis")
 
-static struct option const long_options[] =
-{
-  {NULL, 0, NULL, 0}
-};
-
 /* Token delimiters when reading from a file.  */
 #define DELIM " \t\n"
 
@@ -556,10 +551,8 @@ main (int argc, char **argv)
 
   atexit (close_stdout);
 
-  parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE, Version,
-                      usage, AUTHORS, (char const *) NULL);
-  if (getopt_long (argc, argv, "", long_options, NULL) != -1)
-    usage (EXIT_FAILURE);
+  parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
+                                   true, usage, AUTHORS, (char const *) NULL);
 
   if (1 < argc - optind)
     {
diff --git a/src/unlink.c b/src/unlink.c
index a6ae623f9..16478f780 100644
--- a/src/unlink.c
+++ b/src/unlink.c
@@ -36,11 +36,6 @@
 
 #define AUTHORS proper_name ("Michael Stone")
 
-static struct option const long_options[] =
-{
-  {NULL, 0, NULL, 0}
-};
-
 void
 usage (int status)
 {
@@ -71,10 +66,8 @@ main (int argc, char **argv)
 
   atexit (close_stdout);
 
-  parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
-                      usage, AUTHORS, (char const *) NULL);
-  if (getopt_long (argc, argv, "", long_options, NULL) != -1)
-    usage (EXIT_FAILURE);
+  parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
+                                   true, usage, AUTHORS, (char const *) NULL);
 
   if (argc < optind + 1)
     {
diff --git a/src/uptime.c b/src/uptime.c
index 5f0775069..45e9e4817 100644
--- a/src/uptime.c
+++ b/src/uptime.c
@@ -47,11 +47,6 @@
   proper_name ("David MacKenzie"), \
   proper_name ("Kaveh Ghazi")
 
-static struct option const long_options[] =
-{
-  {NULL, 0, NULL, 0}
-};
-
 static void
 print_uptime (size_t n, const STRUCT_UTMP *this)
 {
@@ -239,10 +234,8 @@ main (int argc, char **argv)
 
   atexit (close_stdout);
 
-  parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
-                      usage, AUTHORS, (char const *) NULL);
-  if (getopt_long (argc, argv, "", long_options, NULL) != -1)
-    usage (EXIT_FAILURE);
+  parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
+                                   true, usage, AUTHORS, (char const *) NULL);
 
   switch (argc - optind)
     {
diff --git a/src/users.c b/src/users.c
index 3287b7a6d..897cec3d8 100644
--- a/src/users.c
+++ b/src/users.c
@@ -36,11 +36,6 @@
   proper_name ("Joseph Arceneaux"), \
   proper_name ("David MacKenzie")
 
-static struct option const long_options[] =
-{
-  {NULL, 0, NULL, 0}
-};
-
 static int
 userid_compare (const void *v_a, const void *v_b)
 {
@@ -133,10 +128,8 @@ main (int argc, char **argv)
 
   atexit (close_stdout);
 
-  parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
-                      usage, AUTHORS, (char const *) NULL);
-  if (getopt_long (argc, argv, "", long_options, NULL) != -1)
-    usage (EXIT_FAILURE);
+  parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
+                                   true, usage, AUTHORS, (char const *) NULL);
 
   switch (argc - optind)
     {
diff --git a/src/whoami.c b/src/whoami.c
index ac2ac0207..86253ac80 100644
--- a/src/whoami.c
+++ b/src/whoami.c
@@ -35,11 +35,6 @@
 
 #define AUTHORS proper_name ("Richard Mlynarik")
 
-static struct option const long_options[] =
-{
-  {NULL, 0, NULL, 0}
-};
-
 void
 usage (int status)
 {
@@ -75,10 +70,8 @@ main (int argc, char **argv)
 
   atexit (close_stdout);
 
-  parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
-                      usage, AUTHORS, (char const *) NULL);
-  if (getopt_long (argc, argv, "", long_options, NULL) != -1)
-    usage (EXIT_FAILURE);
+  parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
+                                   true, usage, AUTHORS, (char const *) NULL);
 
   if (optind != argc)
     {
diff --git a/src/yes.c b/src/yes.c
index 3dd5d2f9d..23659d502 100644
--- a/src/yes.c
+++ b/src/yes.c
@@ -32,11 +32,6 @@
 
 #define AUTHORS proper_name ("David MacKenzie")
 
-static struct option const long_options[] =
-{
-  {NULL, 0, NULL, 0}
-};
-
 void
 usage (int status)
 {
@@ -72,10 +67,8 @@ main (int argc, char **argv)
 
   atexit (close_stdout);
 
-  parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
-                      usage, AUTHORS, (char const *) NULL);
-  if (getopt_long (argc, argv, "+", long_options, NULL) != -1)
-    usage (EXIT_FAILURE);
+  parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
+                                   true, usage, AUTHORS, (char const *) NULL);
 
   char **operands = argv + optind;
   char **operand_lim = argv + argc;
-- 
2.19.1

Reply via email to