summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndreas Metzler <ametzler@bebt.de>2017-10-11 00:47:08 +0200
committerBernhard Voelker <mail@bernhard-voelker.de>2021-01-07 18:28:00 +0100
commitee402bdd3a5b32742e016122c1003c18cff9c8d3 (patch)
tree61986eb902f4d90e98b077c63858f2595fa2ce80
parent62b59ea8b734e72398f388e23c35bd42e6cbc38a (diff)
downloadfindutils-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--NEWS4
-rw-r--r--doc/find.texi28
-rw-r--r--tests/local.mk1
-rwxr-xr-xtests/xargs/conflicting_opts.sh143
-rw-r--r--xargs/testsuite/Makefile.am4
-rw-r--r--xargs/testsuite/config/unix.exp1
-rw-r--r--xargs/testsuite/xargs.gnu/P3-n1-IARG.exp1
-rw-r--r--xargs/testsuite/xargs.gnu/P3-n1-IARG.xe1
-rw-r--r--xargs/testsuite/xargs.posix/L2-n2.exp1
-rw-r--r--xargs/testsuite/xargs.posix/L2-n2.xe1
-rw-r--r--xargs/testsuite/xargs.posix/rc-123.exp1
-rw-r--r--xargs/testsuite/xargs.posix/rc-123.xe1
-rw-r--r--xargs/testsuite/xargs.sysv/l1n4.exp1
-rw-r--r--xargs/testsuite/xargs.sysv/l1n4.xe1
-rw-r--r--xargs/xargs.138
-rw-r--r--xargs/xargs.c68
16 files changed, 277 insertions, 18 deletions
diff --git a/NEWS b/NEWS
index cc74ef1..195f786 100644
--- a/NEWS
+++ b/NEWS
@@ -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