Hello, A follow-up and more details:
On 2019-01-12 11:30 a.m., Assaf Gordon wrote:
On 2019-01-12 8:42 a.m., Eric Blake wrote:On 1/11/19 6:23 PM, Assaf Gordon wrote:- optind = 0; + optind = 1;Ouch. You're hitting the portability problem of the difference between BSD and glibc.I only tested on Debian Stretch (with Debian GLIBC 2.24-11+deb9u3), did not yet test on BSDs. With "optind=1", I see the following: === $ ./src/hostid ec68f06c
[...]
With "optind=0" I see the following: === $ ./src/hostid ./src/hostid: extra operand ‘./src/hostid’ Try './src/hostid --help' for more information.
==== Eric's suggestion was not wrong, "optint=0" was already used (and worked just fine) in parse_long_option. But there's a catch: after calling "parse_long_options" (which sets optind=0), every program called "getopt_long" again! and that call set optind to non-zero value. Bernhard's patch removed the (now unneeded) getopt_long call: === - 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);
===And so all these programs were left with "optind=0" when the checked non-option arguments, e.g.:
===if (optind < argc) { error (0, 0, _("extra operand %s"), quote (argv[optind])); usage (EXIT_FAILURE);
}
===
which resulted in all the parsing errors I reported previously.
Perhaps "parse_gnu_standard_options_only" should use "_getopt_long_r" and avoid the need to reset anything?
_getopt_long_r was ostensibly fine, but turned out to be messy: when coreutils is built on glibc systems, all of gnulib's getopt replacement modules are not used, and so _getopt_long_r is not available. As all the programs in this patch accept only --help and --yes (and non-option arguments), the attached ugly hack seems to solve the issue. There's probably a prettier way.With this patch, the only issues left are nohup's exit code (1 instead of 125) and "dd --", see https://bugs.gnu.org/33468#29
regards, - assaf
>From eb4ed1a5417a2d50941181aa1d8e06b674c661a8 Mon Sep 17 00:00:00 2001 From: Assaf Gordon <[email protected]> Date: Tue, 12 Feb 2019 17:58:47 -0700 Subject: [PATCH] all: parse_gnu_standard_options_only fixup --- src/cksum.c | 1 + src/dd.c | 1 + src/hostid.c | 1 + src/hostname.c | 1 + src/link.c | 1 + src/logname.c | 1 + src/nohup.c | 1 + src/sleep.c | 1 + src/tsort.c | 1 + src/unlink.c | 1 + src/uptime.c | 1 + src/users.c | 1 + src/whoami.c | 1 + src/yes.c | 1 + 14 files changed, 14 insertions(+) diff --git a/src/cksum.c b/src/cksum.c index cda61516a..b62249862 100644 --- a/src/cksum.c +++ b/src/cksum.c @@ -291,6 +291,7 @@ main (int argc, char **argv) parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE, Version, true, usage, AUTHORS, (char const *) NULL); + optind = 1; have_read_stdin = false; diff --git a/src/dd.c b/src/dd.c index b361e7d5a..f47e8a788 100644 --- a/src/dd.c +++ b/src/dd.c @@ -2393,6 +2393,7 @@ main (int argc, char **argv) parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE, Version, true, usage, AUTHORS, (char const *) NULL); + optind = 1; close_stdout_required = false; /* Initialize translation table to identity translation. */ diff --git a/src/hostid.c b/src/hostid.c index d9ea8929b..f023a3da1 100644 --- a/src/hostid.c +++ b/src/hostid.c @@ -66,6 +66,7 @@ main (int argc, char **argv) parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version, true, usage, AUTHORS, (char const *) NULL); + optind = 1; if (optind < argc) { diff --git a/src/hostname.c b/src/hostname.c index 761f775b4..3a9d1dd80 100644 --- a/src/hostname.c +++ b/src/hostname.c @@ -83,6 +83,7 @@ main (int argc, char **argv) parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version, true, usage, AUTHORS, (char const *) NULL); + optind = 1; if (argc == optind + 1) { diff --git a/src/link.c b/src/link.c index d70d434d9..d21f36099 100644 --- a/src/link.c +++ b/src/link.c @@ -69,6 +69,7 @@ main (int argc, char **argv) parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version, true, usage, AUTHORS, (char const *) NULL); + optind = 1; if (argc < optind + 2) { diff --git a/src/logname.c b/src/logname.c index cea43720f..fb9c2bbab 100644 --- a/src/logname.c +++ b/src/logname.c @@ -64,6 +64,7 @@ main (int argc, char **argv) parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version, true, usage, AUTHORS, (char const *) NULL); + optind = 1; if (optind < argc) { diff --git a/src/nohup.c b/src/nohup.c index fa1c5b563..5f62486ce 100644 --- a/src/nohup.c +++ b/src/nohup.c @@ -101,6 +101,7 @@ main (int argc, char **argv) parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version, false, usage, AUTHORS, (char const *) NULL); + optind = 1; if (argc <= optind) { diff --git a/src/sleep.c b/src/sleep.c index af7db8d5c..e0fdc290e 100644 --- a/src/sleep.c +++ b/src/sleep.c @@ -110,6 +110,7 @@ main (int argc, char **argv) parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version, true, usage, AUTHORS, (char const *) NULL); + optind = 1; if (argc == 1) { diff --git a/src/tsort.c b/src/tsort.c index f42fa09ac..e8d43897a 100644 --- a/src/tsort.c +++ b/src/tsort.c @@ -553,6 +553,7 @@ main (int argc, char **argv) parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version, true, usage, AUTHORS, (char const *) NULL); + optind = 1; if (1 < argc - optind) { diff --git a/src/unlink.c b/src/unlink.c index 3b267caa5..e5205c298 100644 --- a/src/unlink.c +++ b/src/unlink.c @@ -68,6 +68,7 @@ main (int argc, char **argv) parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version, true, usage, AUTHORS, (char const *) NULL); + optind = 1; if (argc < optind + 1) { diff --git a/src/uptime.c b/src/uptime.c index d546729a9..099dbc1e9 100644 --- a/src/uptime.c +++ b/src/uptime.c @@ -236,6 +236,7 @@ main (int argc, char **argv) parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version, true, usage, AUTHORS, (char const *) NULL); + optind = 1; switch (argc - optind) { diff --git a/src/users.c b/src/users.c index d4e436e6e..7fa3e6989 100644 --- a/src/users.c +++ b/src/users.c @@ -130,6 +130,7 @@ main (int argc, char **argv) parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version, true, usage, AUTHORS, (char const *) NULL); + optind = 1; switch (argc - optind) { diff --git a/src/whoami.c b/src/whoami.c index 2e0a4016a..8a424ec36 100644 --- a/src/whoami.c +++ b/src/whoami.c @@ -72,6 +72,7 @@ main (int argc, char **argv) parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version, true, usage, AUTHORS, (char const *) NULL); + optind = 1; if (optind != argc) { diff --git a/src/yes.c b/src/yes.c index 9a8f42738..b10e22170 100644 --- a/src/yes.c +++ b/src/yes.c @@ -69,6 +69,7 @@ main (int argc, char **argv) parse_gnu_standard_options_only (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version, true, usage, AUTHORS, (char const *) NULL); + optind = 1; char **operands = argv + optind; char **operand_lim = argv + argc; -- 2.11.0
