On 7/24/19 11:28 PM, Eric Blake wrote:
> Looks reasonable to me (other than line wrapping).

Thanks, attached as patch with a test case, NEWS entry, etc.
Comments?

Have a nice day,
Berny
From eeefd1dc72102c5d8c70912be943a71a0fb153aa Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <m...@bernhard-voelker.de>
Date: Sun, 28 Jul 2019 19:11:15 +0200
Subject: [PATCH] xargs: in -t,-p output, quote the command to be executed when
 needed

To improve readability for the user, xargs(1) shall quote each part
of the command to be executed when printing it to stderr, i.e. for
the -t (--verbose) and -p (--interactive) options.  Example:
  $ echo "some file" | ../xargs -p -I'{}' rm '{}.c' '{}.o'
  rm 'some file.c' 'some file.o' ?...

* xargs/xargs.c (print_args): Apply "shell-escape" quoting.
* xargs/testsuite/xargs.gnu/space-t-0.xe: Adjust test.
* tests/xargs/verbose-quote.sh: Add test.
* tests/local.mk: Reference it.
* README (Improvements): Mention the change.
---
 NEWS                                   |  3 ++
 tests/local.mk                         |  1 +
 tests/xargs/verbose-quote.sh           | 62 ++++++++++++++++++++++++++
 xargs/testsuite/xargs.gnu/space-t-0.xe |  4 +-
 xargs/xargs.c                          |  4 +-
 5 files changed, 70 insertions(+), 4 deletions(-)
 create mode 100755 tests/xargs/verbose-quote.sh

diff --git a/NEWS b/NEWS
index 00895a6b..401d4c51 100644
--- a/NEWS
+++ b/NEWS
@@ -52,6 +52,9 @@ interactive application.  Added for compatibility with BSD.
 xargs now supports GNU_FINDUTILS_FD_LEAK_CHECK environment variable to
 enable/disable fd leak check.
 
+'xargs -t' (--verbose) now properly quotes each part of the command to the
+executed if needed when printing it to stderr; likewise -p (--interactive).
+
 ** Documentation Changes
 
 Prefer https:// over http:// links where possible, e.g. for '*.gnu.org' servers.
diff --git a/tests/local.mk b/tests/local.mk
index ddefe418..7e52a04d 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -115,6 +115,7 @@ all_tests = \
   tests/find/exec-plus-last-file.sh \
   tests/find/refuse-noop.sh \
   tests/find/debug-missing-arg.sh \
+  tests/xargs/verbose-quote.sh \
   $(all_root_tests)
 
 $(TEST_LOGS): $(PROGRAMS)
diff --git a/tests/xargs/verbose-quote.sh b/tests/xargs/verbose-quote.sh
new file mode 100755
index 00000000..75e3e422
--- /dev/null
+++ b/tests/xargs/verbose-quote.sh
@@ -0,0 +1,62 @@
+#!/bin/sh
+# Verify that 'xargs -t' quotes the command properly when needed.
+
+# Copyright (C) 2019 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
+
+# Prepare a command with a whitespace in its file name.
+printf "%s\n" \
+  '#!/bin/sh' \
+  'echo "$@"' \
+  > 'my command' \
+  && chmod +x 'my command' \
+  || framework_failure_
+
+# Run xargs with -t for verious commands which require quoting.
+printf '%s\0' \
+  000 \
+  '10 0' \
+  '20"0' \
+  "30'0" \
+  40$'\n'0 \
+  | xargs -0t '-I{}' './my command' 'hel lo' '{}' world > out 2> err \
+  || fail=1
+
+# Verify stderr.
+# Add trailing blank with sed to avoid FP from sc_trailing_blank.
+sed 's/world/& /' <<\EOF > experr || framework_failure_
+'./my command' 'hel lo' 000 world
+'./my command' 'hel lo' '10 0' world
+'./my command' 'hel lo' '20"0' world
+'./my command' 'hel lo' "30'0" world
+'./my command' 'hel lo' '40'$'\n''0' world
+EOF
+compare experr err || fail=1
+
+# Verify stdout.
+cat <<\EOF > expout || framework_failure_
+hel lo 000 world
+hel lo 10 0 world
+hel lo 20"0 world
+hel lo 30'0 world
+hel lo 40
+0 world
+EOF
+compare expout out || fail=1
+
+Exit $fail
diff --git a/xargs/testsuite/xargs.gnu/space-t-0.xe b/xargs/testsuite/xargs.gnu/space-t-0.xe
index bfcf50bc..e10f8226 100644
--- a/xargs/testsuite/xargs.gnu/space-t-0.xe
+++ b/xargs/testsuite/xargs.gnu/space-t-0.xe
@@ -1,3 +1 @@
-echo this plus that 
- 
	
- 
+echo this plus that ''$'\n'' '$'\f\r\t\v\n' 
diff --git a/xargs/xargs.c b/xargs/xargs.c
index 913328b8..824e9f0b 100644
--- a/xargs/xargs.c
+++ b/xargs/xargs.c
@@ -1093,7 +1093,9 @@ print_args (bool ask)
 
   for (i = 0; i < bc_state.cmd_argc - 1; i++)
     {
-      if (fprintf (stderr, "%s ", bc_state.cmd_argv[i]) < 0)
+      if (fprintf (stderr, "%s ",
+	           quotearg_n_style (0, shell_escape_quoting_style,
+		                     bc_state.cmd_argv[i])) < 0)
 	die (EXIT_FAILURE, errno, _("Failed to write to stderr"));
     }
 
-- 
2.22.0

Reply via email to