Re: [PATCH] quote arguments in xtrace output

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 27/02/2017 07:22, Martijn Dekker wrote:
The xtrace (set -x) output in dash is a bit of a pain, because arguments
containing whitespace aren't quoted. This can it impossible to tell
which bit belongs to which argument:

Agreed.

$ dash -c 'set -x; printf "%s\n" one "two three" four'
+ printf %s\n one two three four
one
two three
four

Another disadvantage is you cannot simply copy and paste the commands
from this xtrace output to repeat them for debugging purposes.

I wrote the attached patch which fixes this. I've been testing it for
about a week and had no issues.

The patch changes the following:

- Since we don't want every command name and argument quoted but only
those containing non-shell-safe characters, single_quote() has acquired
an extra argument indicating whether quoting should be conditional upon
the string containing non-shell-safe characters (1) or unconditional
(0). Shell-safe characters are defined as this set:
0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz%+,./:=@_^!-

- Quoting of variable assignments preceding commands needs to be handled
specially; in order for the output to be suitable for re-entry into the
shell, only the part after the "=" must be quoted. For this purpose, I
changed eprintlist() into two functions, eprintvarlist() to print the
variable assignments and eprintarglist() to print the rest.

The general approach looks nice. I think this misses a few cases though if you're aiming to make it suitable for re-entry:

  $ src/dash -c 'set -x; \!'
  + !
  src/dash: 1: !: not found

  $ src/dash -c 'set -x; \a=b'
  + a=b
  src/dash: 1: a=b: not found

  $ src/dash -c 'set -x; \if'
  + if
  src/dash: 1: if: not found

I could create binaries by those names to get rid of the errors. Regardless, in none these cases would the original command be re-executed if the set -x output were entered into the shell.

Aside from the first, bash doesn't quote them either. It may be okay to leave this as it is and acknowledge that it doesn't handle all cases, as long as it handles enough of them to be an improvement.

Depending on what happens with <http://austingroupbugs.net/view.php?id=249>, if support for $'...' will become required in a future version of POSIX, it may additionally make sense to use that syntax for control characters already right now.

It feels like the set of shell-safe characters should be able to use the generated syntax.[ch] files, but none of the existing categories quite matches.

If the modification to single_quote can handle all cases (perhaps quoting unnecessarily if it's unclear), then the conditional parameter could be removed and applied unconditionally: as far as I can tell, there are no existing calls that require the single quotes for simple words.

Hope this is useful,

- Martijn

It looks useful to me. :)

Cheers,
Harald van Dijk
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [LARTC]     [Bugtraq]     [Yosemite Forum]     [Photo]

  Powered by Linux