diff options
| author | Andreas Metzler <ametzler@bebt.de> | 2017-10-11 00:47:08 +0200 |
|---|---|---|
| committer | Bernhard Voelker <mail@bernhard-voelker.de> | 2021-01-07 18:28:00 +0100 |
| commit | ee402bdd3a5b32742e016122c1003c18cff9c8d3 (patch) | |
| tree | 61986eb902f4d90e98b077c63858f2595fa2ce80 | |
| parent | 62b59ea8b734e72398f388e23c35bd42e6cbc38a (diff) | |
| download | findutils-ee402bdd.tar.gz | |
xargs: warn on conflicting options -L, -I, -n
* xargs/xargs.c (warn_mutually_exclusive): Add function to output the
new warning diagnostic.
(main): Call the above new function in order to show a warning message
if conflicting options (-L -l -I -i -n) are specified.
* xargs/xargs.1: Explicitly document the fact that -L -l -I -i -n
options are mutually exclusive and how xargs behaves when more than one
of these options are specified.
* doc/find.texi (node Conflicting xargs options): Likewise, add this
section with examples.
* xargs/testsuite/config/unix.exp (xargs_start): Replace the full path
of the xargs executable in the stderr output file by its basename.
* xargs/testsuite/xargs.gnu/P3-n1-IARG.xe: Add file containing the
expected output with the new warning diagnostic.
* xargs/testsuite/xargs.posix/L2-n2.xe: Likewise.
* xargs/testsuite/xargs.posix/rc-123.xe: Likewise.
* xargs/testsuite/xargs.sysv/l1n4.xe: Likewise.
* xargs/testsuite/xargs/testsuite/xargs.gnu/P3-n1-IARG.exp: Ensure the
error diagnostic is in C locale to avoid false positives in other locales.
* xargs/testsuite/xargs.posix/L2-n2.exp: Likewise.
* xargs/testsuite/xargs.posix/rc-123.exp: Likewise.
* xargs/testsuite/xargs.sysv/l1n4.exp: Likewise.
* tests/xargs/conflicting_opts.sh: Add test coverage for combinations
of the mutually exclusive options -I -L and -n.
* xargs/testsuite/Makefile.am (EXTRA_DIST_EXP): Reference the new tests.
(EXTRA_DIST_XO): Reference the new *.xo files.
(EXTRA_DIST_XE): Reference the new *.xe files.
* NEWS (Improvements): Mention the change.
Co-authored-by: Bernhard Voelker <mail@bernhard-voelker.de>
Discussed at https://savannah.gnu.org/bugs/?52137
Duplicate of https://savannah.gnu.org/bugs/?58156
| -rw-r--r-- | NEWS | 4 | ||||
| -rw-r--r-- | doc/find.texi | 28 | ||||
| -rw-r--r-- | tests/local.mk | 1 | ||||
| -rwxr-xr-x | tests/xargs/conflicting_opts.sh | 143 | ||||
| -rw-r--r-- | xargs/testsuite/Makefile.am | 4 | ||||
| -rw-r--r-- | xargs/testsuite/config/unix.exp | 1 | ||||
| -rw-r--r-- | xargs/testsuite/xargs.gnu/P3-n1-IARG.exp | 1 | ||||
| -rw-r--r-- | xargs/testsuite/xargs.gnu/P3-n1-IARG.xe | 1 | ||||
| -rw-r--r-- | xargs/testsuite/xargs.posix/L2-n2.exp | 1 | ||||
| -rw-r--r-- | xargs/testsuite/xargs.posix/L2-n2.xe | 1 | ||||
| -rw-r--r-- | xargs/testsuite/xargs.posix/rc-123.exp | 1 | ||||
| -rw-r--r-- | xargs/testsuite/xargs.posix/rc-123.xe | 1 | ||||
| -rw-r--r-- | xargs/testsuite/xargs.sysv/l1n4.exp | 1 | ||||
| -rw-r--r-- | xargs/testsuite/xargs.sysv/l1n4.xe | 1 | ||||
| -rw-r--r-- | xargs/xargs.1 | 38 | ||||
| -rw-r--r-- | xargs/xargs.c | 68 |
16 files changed, 277 insertions, 18 deletions
@@ -4,6 +4,10 @@ GNU findutils NEWS - User visible changes. -*- outline -*- (allout) ** Changes in xargs +xargs now warns when more than one of the conflicting options --max-lines (-L, +-l), --replace (-i/-I) and --max-args (-n) are specified on the command line. +[#52137] + 'xargs -t' no longer outputs a trailing blank to stderr after the last argument of each constructed command line to be executed. [#57291] diff --git a/doc/find.texi b/doc/find.texi index f8bfcba..fbf5918 100644 --- a/doc/find.texi +++ b/doc/find.texi @@ -3742,6 +3742,7 @@ a program died due to a fatal signal. @menu * xargs options:: +* Conflicting xargs options:: * Invoking the shell from xargs:: @end menu @@ -3891,6 +3892,33 @@ integer. Values are reused once child processes exit. This can be used in a rudimentary load distribution scheme, for example. @end table +@node Conflicting xargs options +@subsection Conflicting options +The options @samp{--max-lines} (@samp{-L}, @samp{-l}), @samp{--replace} +(@samp{-I}, @samp{-i}) and @samp{--max-args} (@samp{-n}) are mutually exclusive. + +If some of them are specified at the same time, then @code{xargs} will +generally use the option specified last on the command line, i.e., it will +reset the value of the offending option (given before) to its default value. +Additionally, @code{xargs} will issue a warning diagnostic on @file{stderr}. + +@example +$ seq 4 | xargs -l2 -n3 +xargs: warning: options --max-lines and --max-args/-n are \ + mutually exclusive, ignoring previous --max-lines value +1 2 3 +4 +@end example + +The exception to this rule is that the special @var{max-args} value @var{1} is +ignored after the @samp{--replace} option and its short-option aliases @samp{-I} +and @samp{-i}, because it would not actually conflict. +@example +$ seq 2 | xargs --replace -n1 echo a-@{@}-b +a-1-b +a-2-b +@end example + @node Invoking the shell from xargs @subsection Invoking the shell from xargs diff --git a/tests/local.mk b/tests/local.mk index 6e67bad..95bc3df 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -117,6 +117,7 @@ all_tests = \ tests/find/refuse-noop.sh \ tests/find/debug-missing-arg.sh \ tests/find/used.sh \ + tests/xargs/conflicting_opts.sh \ tests/xargs/verbose-quote.sh \ $(all_root_tests) diff --git a/tests/xargs/conflicting_opts.sh b/tests/xargs/conflicting_opts.sh new file mode 100755 index 0000000..bfdfe9d --- /dev/null +++ b/tests/xargs/conflicting_opts.sh @@ -0,0 +1,143 @@ +#!/bin/sh +# Test mutually exclusive options --max-args, --max-lines, --replace, and +# their short-option aliases (-L, -l -I -i -n). + +# Copyright (C) 2021 Free Software Foundation, Inc. + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <https://www.gnu.org/licenses/>. + +. "${srcdir=.}/tests/init.sh" +print_ver_ xargs + +# The idea behind: when using the mutually exclusive options, the last one +# specified overrides the values of the previous ones. Or to put it in +# other words: the resulting output (and exit code btw.) is the same as if +# the earlier, conflicting options had not been passed - apart from the +# error diagnostic for every overridden option. + +# Create some input file. +cat <<\EOF > in || framework_failure_ +a b c +d e f +g h i +EOF + +WARNFMT='xargs: warning: options %s and %s are mutually exclusive, ignoring previous %s value' + +str4opt() { + case "$1" in + -n*|--max-args*) echo '--max-args/-n';; + -l*|--max-lines*) echo '--max-lines/-l';; + -i|-I*|--replace*) echo '--replace/-I/-i';; + esac +} +lopt4opt() { + case "$1" in + -n*|--max-args*) echo '--max-args';; + -l*|--max-lines*) echo '--max-lines';; + -i|-I*|--replace*) echo '--replace';; + esac +} + +gen_err() { # see warn_mutually_exclusive in xargs.c. + s="$(str4opt "$1")" || framework_failure_ + l="$(lopt4opt "$2")" || framework_failure_ + + printf "$WARNFMT\n" "$l" "$s" "$l" +} + +# test_xargs a test case, using $1 as the last option which should finally override +# any other of the mutually exclusive options. +# Example: test_xargs -n2 OTHER_OPTION +# This will invoke 'xargs' with that option as the last one: +# xargs OTHER_OPTION -n2 +test_xargs() { + rm -f out err err2 expout experr || framework_failure_ + + # Generate the expected output file for the option in "$1". + # test_xargs xargs with that option only; stderr should be empty + # and stdout of this test_xargs is the reference for the actual test. + last_opt="$1" \ + && shift 1 \ + && xargs "$last_opt" < in > expout 2>err \ + && compare /dev/null err \ + || framework_failure_ + + # Generate the expected error diagnostic for the mutually exclusive option + # passed earlier. No diagnostic if the previous mutually-exclusive option + # is the same as the current one. + prev_excl_option= # variable for previous mutually-exclusive option. + for o in "$@" "$last_opt"; do + lopt="$(lopt4opt "$o")" || framework_failure_ + case $o in -n*|--max-args*|-l*|--max-lines*|-i|-I*|--replace*) + test "$prev_excl_option" != '' \ + && test "$prev_excl_option" != "$lopt" \ + && gen_err "$o" "$prev_excl_option" + prev_excl_option="$lopt" + ;; + esac + done > experr + + # test_xargs the test. + xargs "$@" "$last_opt" < in > out 2>err || fail=1 + sed 's/^.*\(xargs:\)/&/' err > err2 || framework_failure_ + compare expout out || fail=1 + compare experr err2 || fail=1 +} + +# test_xargs the tests for --max-args as last option. +for last_opt in -n2 --max-args=2; do + for other_opt in --max-lines=2 -l2 --replace='REPL' -I'REPL' -i; do + test_xargs $last_opt $other_opt + done +done + +# test_xargs the tests for --max-line as last option. +for last_opt in -l2 --max-lines=2; do + for other_opt in --max-args=2 -n2 --replace='REPL' -I'REPL' -i; do + test_xargs $last_opt $other_opt + done +done + +# test_xargs the tests for --replace as last option. +for last_opt in --replace='REPL' -I'REPL' -i; do + for other_opt in --max-args=2 -n2 -l2 --max-lines=2; do + test_xargs $last_opt $other_opt + done +done + +# Finally do some multi-option tests. +# Test 'xargs -l2 -l3 -l4 -i -n2': +# diagnostic for -l once, then for -i being overridden. +test_xargs -n2 -l2 -l3 -l4 -i + +# Test 'xargs -n2 -i -l4 -l5': diagnostic for -n, then once for -i. +test_xargs -l5 -n2 -i -l4 + +# Test 'xargs -l2 --max-lines=3 --replace=4 -I5 --max-args=6 -n7 -n1': +# Diagnostic for the change from MAX-LINES to REPLACE, +# then from REPLACE to MAX-ARGS. +test_xargs -n1 -l2 --max-lines=3 --replace=4 -I5 --max-args=6 -n7 + +# Test 'xargs --replace=REPL -n1': verify that it doesn't emit the warning. +# See discussion at: https://sv.gnu.org/patch/?1500 +xargs -I_ echo 'a._.b' < in > expout 2>err \ + && compare /dev/null err \ + || framework_failure_ + +xargs -I_ -n1 echo 'a._.b' < in > out 2>err || fail=1 +compare expout out || fail=1 +compare /dev/null err || fail=1 + +Exit $fail diff --git a/xargs/testsuite/Makefile.am b/xargs/testsuite/Makefile.am index 307bab2..8d85d18 100644 --- a/xargs/testsuite/Makefile.am +++ b/xargs/testsuite/Makefile.am @@ -213,9 +213,13 @@ xargs.sysv/trace.xo EXTRA_DIST_XE = \ +xargs.gnu/P3-n1-IARG.xe \ xargs.gnu/space-t-0.xe \ +xargs.posix/L2-n2.xe \ +xargs.posix/rc-123.xe \ xargs.sysv/empty_def-t.xe \ xargs.sysv/empty-t.xe \ +xargs.sysv/l1n4.xe \ xargs.sysv/s25-t.xe \ xargs.sysv/space-t.xe \ xargs.sysv/trace.xe diff --git a/xargs/testsuite/config/unix.exp b/xargs/testsuite/config/unix.exp index 52fc7c3..7dca893 100644 --- a/xargs/testsuite/config/unix.exp +++ b/xargs/testsuite/config/unix.exp @@ -157,6 +157,7 @@ proc xargs_start { passfail options {infile ""} {errh ""} {command ""} } { # if stderr check is enabled, if {$errfile != ""} then { if {[file exists $errfile]} then { + catch "exec sed -i s/^.*xargs:/xargs:/ xargs.err" tmp set cmp_cmd "cmp xargs.err $errfile" send_log "$cmp_cmd\n" catch "exec $cmp_cmd" cmperr diff --git a/xargs/testsuite/xargs.gnu/P3-n1-IARG.exp b/xargs/testsuite/xargs.gnu/P3-n1-IARG.exp index 7579589..5ff49d8 100644 --- a/xargs/testsuite/xargs.gnu/P3-n1-IARG.exp +++ b/xargs/testsuite/xargs.gnu/P3-n1-IARG.exp @@ -1 +1,2 @@ +set env(LC_ALL) "C" xargs_start p {-P3 -n1 -IARG sh -c ARG} Pdata.xi diff --git a/xargs/testsuite/xargs.gnu/P3-n1-IARG.xe b/xargs/testsuite/xargs.gnu/P3-n1-IARG.xe new file mode 100644 index 0000000..b07e415 --- /dev/null +++ b/xargs/testsuite/xargs.gnu/P3-n1-IARG.xe @@ -0,0 +1 @@ +xargs: warning: options --max-args and --replace/-I/-i are mutually exclusive, ignoring previous --max-args value diff --git a/xargs/testsuite/xargs.posix/L2-n2.exp b/xargs/testsuite/xargs.posix/L2-n2.exp index 88a939f..9a979cc 100644 --- a/xargs/testsuite/xargs.posix/L2-n2.exp +++ b/xargs/testsuite/xargs.posix/L2-n2.exp @@ -1 +1,2 @@ +set env(LC_ALL) "C" xargs_start p {-L2 -n2} ldata.xi diff --git a/xargs/testsuite/xargs.posix/L2-n2.xe b/xargs/testsuite/xargs.posix/L2-n2.xe new file mode 100644 index 0000000..f1983b9 --- /dev/null +++ b/xargs/testsuite/xargs.posix/L2-n2.xe @@ -0,0 +1 @@ +xargs: warning: options --max-lines and --max-args/-n are mutually exclusive, ignoring previous --max-lines value diff --git a/xargs/testsuite/xargs.posix/rc-123.exp b/xargs/testsuite/xargs.posix/rc-123.exp index 217de0f..1bcf490 100644 --- a/xargs/testsuite/xargs.posix/rc-123.exp +++ b/xargs/testsuite/xargs.posix/rc-123.exp @@ -1 +1,2 @@ +set env(LC_ALL) "C" xargs_start 123 {-n1 -IARG sh -c ARG} ftt.xi diff --git a/xargs/testsuite/xargs.posix/rc-123.xe b/xargs/testsuite/xargs.posix/rc-123.xe new file mode 100644 index 0000000..b07e415 --- /dev/null +++ b/xargs/testsuite/xargs.posix/rc-123.xe @@ -0,0 +1 @@ +xargs: warning: options --max-args and --replace/-I/-i are mutually exclusive, ignoring previous --max-args value diff --git a/xargs/testsuite/xargs.sysv/l1n4.exp b/xargs/testsuite/xargs.sysv/l1n4.exp index 4eef466..26a586f 100644 --- a/xargs/testsuite/xargs.sysv/l1n4.exp +++ b/xargs/testsuite/xargs.sysv/l1n4.exp @@ -1 +1,2 @@ +set env(LC_ALL) "C" xargs_start p {-l1 -n4} files.xi diff --git a/xargs/testsuite/xargs.sysv/l1n4.xe b/xargs/testsuite/xargs.sysv/l1n4.xe new file mode 100644 index 0000000..f1983b9 --- /dev/null +++ b/xargs/testsuite/xargs.sysv/l1n4.xe @@ -0,0 +1 @@ +xargs: warning: options --max-lines and --max-args/-n are mutually exclusive, ignoring previous --max-lines value diff --git a/xargs/xargs.1 b/xargs/xargs.1 index d5735c7..66dcfde 100644 --- a/xargs/xargs.1 +++ b/xargs/xargs.1 @@ -287,6 +287,38 @@ and exit. Print the version number of .B xargs and exit. +.PP +The options +.B \-\-max-lines +(\fB\-L\fP, \fB\-l\fP), +.B \-\-replace +(\fB\-I\fP, \fB\-i\fP) +and +.B \-\-max-args +(\fB\-n\fP) +are mutually exclusive. If some of them are specified at the same +time, then +.B xargs +will generally use the option specified last on the command line, +i.e., it will reset the value of the offending option (given before) +to its default value. +Additionally, +.B xargs +will issue a warning diagnostic on +.IR stderr . +The exception to this rule is that the special +.I max-args +value +.I 1 +('\fB\-n\fP\fI1\fP') +is ignored after the +.B \-\-replace +option and its aliases +.B \-I +and +.BR \-i , +because it would not actually conflict. + . .SH "EXAMPLES" .nf @@ -383,12 +415,6 @@ option can be used to discover the actual limits in force on the current system. . .SH "BUGS" -The -.B \-L -option is incompatible with the -.B \-I -option, but perhaps should not be. -.P It is not possible for .B xargs to be used securely, since there will always be a time gap between the diff --git a/xargs/xargs.c b/xargs/xargs.c index 64066d3..7e3db67 100644 --- a/xargs/xargs.c +++ b/xargs/xargs.c @@ -386,6 +386,14 @@ static FILE* fopen_cloexec_for_read_only (const char *file_name) } +static void warn_mutually_exclusive (const char *option, const char *offending) +{ + error (0, 0, _("warning: options %s and %s are mutually exclusive, " + "ignoring previous %s value"), + offending, option, offending); +} + + int main (int argc, char **argv) { @@ -544,15 +552,31 @@ main (int argc, char **argv) else bc_ctl.replace_pat = "{}"; /* -i excludes -n -l. */ - bc_ctl.args_per_exec = 0; - bc_ctl.lines_per_exec = 0; + if (bc_ctl.args_per_exec != 0) + { + warn_mutually_exclusive ("--replace/-I/-i", "--max-args"); + bc_ctl.args_per_exec = 0; + } + if (bc_ctl.lines_per_exec != 0) + { + warn_mutually_exclusive ("--replace/-I/-i", "--max-lines"); + bc_ctl.lines_per_exec = 0; + } break; case 'L': /* POSIX */ bc_ctl.lines_per_exec = parse_num (optarg, 'L', 1L, -1L, 1); /* -L excludes -i -n. */ - bc_ctl.args_per_exec = 0; - bc_ctl.replace_pat = NULL; + if (bc_ctl.args_per_exec != 0) + { + warn_mutually_exclusive ("-L", "--max-args"); + bc_ctl.args_per_exec = 0; + } + if (bc_ctl.replace_pat != NULL) + { + warn_mutually_exclusive ("-L", "--replace"); + bc_ctl.replace_pat = NULL; + } break; case 'l': /* deprecated */ @@ -561,19 +585,39 @@ main (int argc, char **argv) else bc_ctl.lines_per_exec = 1; /* -l excludes -i -n. */ - bc_ctl.args_per_exec = 0; - bc_ctl.replace_pat = NULL; + if (bc_ctl.args_per_exec != 0) + { + warn_mutually_exclusive ("--max-lines/-l", "--max-args"); + bc_ctl.args_per_exec = 0; + } + if (bc_ctl.replace_pat != NULL) + { + warn_mutually_exclusive ("--max-lines/-l", "--replace"); + bc_ctl.replace_pat = NULL; + } break; case 'n': bc_ctl.args_per_exec = parse_num (optarg, 'n', 1L, -1L, 1); /* -n excludes -i -l. */ - bc_ctl.lines_per_exec = 0; - if (bc_ctl.args_per_exec == 1 && bc_ctl.replace_pat) - /* ignore -n1 in '-i -n1' */ - bc_ctl.args_per_exec = 0; - else - bc_ctl.replace_pat = NULL; + if (bc_ctl.lines_per_exec != 0) + { + warn_mutually_exclusive ("--max-args/-n", "--max-lines"); + bc_ctl.lines_per_exec = 0; + } + if (bc_ctl.replace_pat != NULL) + { + if (bc_ctl.args_per_exec == 1) + { + /* ignore -n1 in '-i -n1' - https://sv.gnu.org/patch/?1500 */ + bc_ctl.args_per_exec = 0; + } + else + { + warn_mutually_exclusive ("--max-args/-n", "--replace"); + bc_ctl.replace_pat = NULL; + } + } break; /* The POSIX standard specifies that it is not an error |
