Project

General

Profile

Bug #4857

xargs(1) -n and -I combine to do potentially the wrong thing

Added by Rich Lowe almost 6 years ago. Updated almost 6 years ago.

Status:
Closed
Priority:
Normal
Category:
cmd - userland programs
Start date:
2014-05-09
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:

Description

We do these two things, which seems clearly wrong:

$ echo foo bar baz other | /bin/xargs -n 1 -I THING echo '** THING **'
** other ** foo bar baz

$ echo foo bar baz other | /bin/xargs -I THING -n 1 echo '** THING **'
** THING ** foo
** THING ** bar
** THING ** baz
** THING ** other

GNU does (regardless the order of the -n and -I options)

$ echo foo bar baz other | /usr/gnu/bin/xargs -n 1 -I THING echo '** THING **'
** foo bar baz other **

Which isn't what I expected either.

OS X does:

echo foo bar baz other | /usr/bin/xargs -n 1 -I THING echo '** THING **'
** foo **
** bar **
** baz **
** other **

Which is what I expected.

I'd say it's obvious that what our xargs(1) is doing is wrong. I'm not certain if there's some weird reason that what GNU xargs is doing is correct, if unexpected, as compared to the OS X behaviour though.

I'd expect that (value of -n) options would be pasted where THING is, so that we'd get the OSX behaviour where -n 1, two items where -n 2, etc. The default being such that they all paste there, with so few options. Whether the standards and whatnot agree with me I don't know.

History

#1

Updated by Jason King almost 6 years ago

http://pubs.opengroup.org/onlinepubs/009604599/utilities/xargs.html seems to suggest -I implies the command given is executed once per line of input (with the entire line treated as a single argument), replacing text as specified. This suggests the GNU behavior might be the most POSIX-ly compliant behavior.

#2

Updated by Garrett D'Amore almost 6 years ago

GNU's behavior is correct. Ours isn't. I think I regressed the behavior.

The specific language POSIX requires is that -I treats the whole line as input. Goofy, I know.

#3

Updated by Garrett D'Amore almost 6 years ago

  • Assignee set to Garrett D'Amore
  • % Done changed from 0 to 50

Actually, -n1 and -I are mutually incompatible semantically.

The behavior of xargs is to try to apply the last behavior.

Probably what should happen is that if -I is specified last, it should cancel -n. And vice versa.

We almost have that behavior correct. The bug is that -I THING doesn't properly cancel out -n 1 (but if -n is specified last it replaces the -I THING behavior).

I'll take this.

#4

Updated by Garrett D'Amore almost 6 years ago

Actually this isn't quite right analysis. Because -I is busted even without -n.

#5

Updated by Garrett D'Amore almost 6 years ago

--- a/usr/src/cmd/xargs/xargs.c
+++ b/usr/src/cmd/xargs/xargs.c
@ -19,6 +19,7 @ * CDDL HEADER END
/
/

+ * Copyright 2014 Garrett D'Amore <> * Copyright 2012 DEY Storage Systems, Inc. All rights reserved. * * Portions of this file developed by DEY Storage Systems, Inc. are licensed
@ -421,8 +422,8 @ main(int argc, char **argv)

N_args++;

- if ((PER_LINE && N_lines >= PER_LINE) ||
- (N_ARGS && (N_args) >= N_ARGS)) {
+ if ((PER_LINE && (N_lines >= PER_LINE)) ||
+ (N_ARGS && (N_args >= N_ARGS))) {
break;
}

@ -612,7 +613,8 @ getarg(char *arg)

default:
/* most times we will just want to store it */
- if (inquote || escape || ZERO || !iswctype(c, blank)) {
+ if (inquote || escape || ZERO || PER_LINE ||
+ !iswctype(c, blank)) {
escape = 0;
store_str(&arg, mbc, len);
continue;
#6

Updated by Garrett D'Amore almost 6 years ago

  • Status changed from New to Pending RTI
  • % Done changed from 50 to 90
#8

Updated by Electric Monk almost 6 years ago

  • Status changed from Pending RTI to Closed
  • % Done changed from 90 to 100

git commit 826ac02a0def83e0a41b29321470d299c7389aab

commit  826ac02a0def83e0a41b29321470d299c7389aab
Author: Garrett D'Amore <garrett@damore.org>
Date:   2014-05-20T14:50:37.000Z

    4857 xargs(1) -n and -I combine to do potentially the wrong thing
    Reviewed by: Robert Mustacchi <rm@joyent.com>
    Approved by: Dan McDonald <danmcd@omniti.com>

Also available in: Atom PDF