bug#34522: Bug in --help command of comm --help
tag 34522 notabug close 34522 stop On 17/02/19 22:17, Sai Bhargav Varanasi wrote: > Examples: > comm -12 file1 file2 Print only lines present in both file1 and file2. > comm -3 file1 file2 Print lines in file1 not in file2, and vice versa. > > comm -3 prints lines that are unique to both file1 and file2 in separate > columns. > comm *-23* prints the lines that are in file1 and not in file2. You seem to have missed the "and vice versa" part. I was going for a concise description that was from an alternative viewpoint to the summary in the descriptions. > I hope the correction will be made soon enough. Generally statements like that are at best redundant. thanks, Pádraig.
bug#34583: minor documentation error (info page)
forcemerge 34583 34584 close 34583 stop On 19/02/19 16:20, Martin Castillo wrote: > Hi, > > I found an error in the info page for version 8.29. > > As linked below, in the join(1) documentation, there is the following: > > $ join -v 1 file1 file2 unpaired lines from the first file > b 1 (_difference_) > > it should output > b 2 Pushed these fixes in your name at: https://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=f473489 thanks! Pádraig
bug#34584: documentation error: invalid command
Hi, I found an error in the info page for version 8.29. As linked below, in the join(1) documentation, there are four lines starting with: join -t'' it should be: join -t '' Martin Castillo [1]: https://www.gnu.org/software/coreutils/manual/html_node/Set-operations.html#Set-operations -- GPG: 7FDE 7190 2F73 2C50 236E 403D CC13 48F1 E644 08EC
bug#34583: minor documentation error (info page)
Hi, I found an error in the info page for version 8.29. As linked below, in the join(1) documentation, there is the following: $ join -v 1 file1 file2 unpaired lines from the first file b 1 (_difference_) it should output b 2 Martin Castillo [1]: https://www.gnu.org/software/coreutils/manual/html_node/Paired-and-unpaired-lines.html#Paired-and-unpaired-lines -- GPG: 7FDE 7190 2F73 2C50 236E 403D CC13 48F1 E644 08EC
bug#33468: A bug with yes and --help
On 2/19/19 2:24 AM, Bernhard Voelker wrote: > On 2/18/19 11:20 AM, Assaf Gordon wrote: >> [...] what do you think ? > > Thanks for driving this. > > To Eric's suggestion, I'd remove the RESET_OPTIND function argument, > because it's never used. > Re. OPTIND: what about resetting the values of all involved externals > to their previous value? > > + int saved_opterr = opterr; > + int saved_optind = optind; > + int saved_optopt = optopt; > + char *saved_optarg = optarg; > > ... > > + /* Restore previous values. */ > + opterr = saved_opterr; > + optind = saved_optind; > + optopt = saved_optopt; > + optarg = saved_optarg; Why? Most callers call this _instead_ of getopt_long(), so they don't care what things are left at except that optind must point to the first non-option. Restoring optind (to 1) is wrong. And for the rare caller that DOES want to call getopt_long() themselves afterwards, they can safe/restore as needed prior to calling this helper. Restoring state made sense as long as we had a RESET_OPTIND argument, but now I'm thinking that it does not buy us any utility at all - just blindly set opterr without worrying about restoring it. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
bug#33468: A bug with yes and --help
Hello, On 2019-02-19 1:24 a.m., Bernhard Voelker wrote: On 2/18/19 11:20 AM, Assaf Gordon wrote: [...] what do you think ? To Eric's suggestion, I'd remove the RESET_OPTIND function argument, because it's never used. +1 Re. OPTIND: what about resetting the values of all involved externals to their previous value? + int saved_optind = optind; ... + /* Restore previous values. */ + optind = saved_optind; I believe restoring optind is incorrect here - did the tests pass after this change ? For example, I get the following: --- $ ./src/dd -- ./src/dd: unrecognized operand ‘--’ Try './src/dd --help' for more information. $ ./src/nohup -- ./src/nohup: ignoring input and appending output to 'nohup.out' ./src/nohup: failed to run command '--': No such file or directory $ ./src/yes -- | head -n1 -- --- All these programs expect 'optind' to point to the first non-option argument (because they all called "getopt_long" directly before your patch, and parse_gnu_standard_options_only() now calls getopt_long() for them, indirectly). So restoring it to its initial value of 1 is going to confuse the programs when they look into argv[optind] . Unless I got confused (it's rather late here). I'll double-check in the morning. regards, - assaf
bug#33468: A bug with yes and --help
On 2/18/19 11:20 AM, Assaf Gordon wrote:
> [...] what do you think ?
Thanks for driving this.
To Eric's suggestion, I'd remove the RESET_OPTIND function argument,
because it's never used.
Re. OPTIND: what about resetting the values of all involved externals
to their previous value?
+ int saved_opterr = opterr;
+ int saved_optind = optind;
+ int saved_optopt = optopt;
+ char *saved_optarg = optarg;
...
+ /* Restore previous values. */
+ opterr = saved_opterr;
+ optind = saved_optind;
+ optopt = saved_optopt;
+ optarg = saved_optarg;
In the attached, I've also changed the test a bit:
- remove unused _cleanup().
- add 'print_ver_ yes' to satisfy sc_env_test_dependencies,
- use case/esac to avoid spawning expr for the matching,
- and fix a missing "|| fail=1" in the final 'yes' test.
*BUT*: for 'yes', we'd introduce a regression due to option reordering:
$ /usr/bin/yes a -- --help | head -n1
a -- --help
$ src/yes a -- --help | head -n1
-- a --help
Any idea?
Thanks & have a nice day,
Berny
>From 0703d2520f79b499385d2f0617aac15203377415 Mon Sep 17 00:00:00 2001
From: Bernhard Voelker
Date: Mon, 26 Nov 2018 09:05:37 +0100
Subject: [PATCH] all: detect --help and --version more consistently
For select programs which accept only --help and --version options
(in addition to non-option arguments), process these options before
any other options.
Before:
$ dd bs=1 --help
dd: unrecognized option '--help'
Try 'dd --help' for more information.
$ yes me --help
me --help
me --help
...
After:
Any occurrence of '--help' in the arguments (prior to '--') will
show the help screen.
Discussed in https://bugs.gnu.org/33468 .
* NEWS: Mention change.
* src/cksum.c, src/dd.c, src/hostid.c, src/hostname.c, src/link.c,
src/logname.c, src/nohup.c, src/sleep.c, src/tsort.c, src/unlink.c,
src/uptime.c, src/users.c, src/whoami.c, src/yes.c (main): Replace
parse_long_options() + getopt_long() calls with
parse_gnu_standard_options_only(); Remove inclusion;
Remove empty 'struct long_options' variable;
* tests/misc/help-version-getopt.sh: New test.
* tests/local.mk (all_tests): Add new test.
---
NEWS | 8 +
src/cksum.c | 12 ++-
src/dd.c | 13 ++-
src/hostid.c | 13 ++-
src/hostname.c| 13 ++-
src/link.c| 13 ++-
src/logname.c | 13 ++-
src/nohup.c | 13 ++-
src/sleep.c | 13 ++-
src/tsort.c | 13 ++-
src/unlink.c | 13 ++-
src/uptime.c | 13 ++-
src/users.c | 13 ++-
src/whoami.c | 13 ++-
src/yes.c | 13 ++-
tests/local.mk| 1 +
tests/misc/help-version-getopt.sh | 57 +++
17 files changed, 106 insertions(+), 141 deletions(-)
create mode 100755 tests/misc/help-version-getopt.sh
diff --git a/NEWS b/NEWS
index fdde47593..5757602d6 100644
--- a/NEWS
+++ b/NEWS
@@ -37,6 +37,14 @@ GNU coreutils NEWS-*- outline -*-
** Changes in behavior
+ cksum, dd, hostid, hostname, link, logname, sleep, tsort, unlink,
+ uptime, users, whoami, yes: now always process --help and --version options,
+ regardless of any other arguments present before any optional '--'
+ end-of-options marker.
+
+ nohup: now process --help and --version as first options even if other
+ parameters follow.
+
echo now always processes backslash escapes when the POSIXLY_CORRECT
environment variable is set.
diff --git a/src/cksum.c b/src/cksum.c
index 6974724ed..c60510b4f 100644
--- a/src/cksum.c
+++ b/src/cksum.c
@@ -102,16 +102,10 @@ main (void)
#else /* !CRCTAB */
-# include
# include "long-options.h"
# 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 +288,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 28611b3e6..2888b8e33 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -22,7 +22,6 @@
#include
#include
-#include
#include "system.h"
#include "close-stream.h"
@@ -46,11 +45,6 @@
proper_name ("David MacKenzie"), \
proper_name ("Stuart
