Page MenuHomeFreeBSD
Differential D3330

fix regression in xargs -Px and add regression tests
ClosedPublic
Actions

Authored by allanjude on Aug 7 2015, 1:35 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 18, 10:56 PM
Unknown Object (File)
Fri, Apr 18, 10:44 PM
Unknown Object (File)
Fri, Apr 18, 10:06 PM
Unknown Object (File)
Fri, Apr 18, 9:56 PM
Unknown Object (File)
Sun, Apr 6, 12:08 PM
Unknown Object (File)
Tue, Apr 1, 5:36 PM
Unknown Object (File)
Mar 16 2025, 2:06 AM
Unknown Object (File)
Feb 21 2025, 7:13 PM
View All Files
Subscribers
imp

Details

Reviewers
lifanov_mail.lifanov.com
marcel
bapt
jbeich
Commits
rS286461: fix regression in xargs -Px (introduced in r286289) and add regression tests
Summary

jbeich reported that xargs -Px doesn't work anymore because
of a botched rebase. This is true, but easy to fix. See attached patch.
I also added test cases to xargs to test -P functionality in the future.

There is Bugzilla 202152 about this.

Test Plan

kyua -k /usr/tests usr.bin/xargs

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

lifanov_mail.lifanov.com updated this revision to Diff 7754.Aug 7 2015, 1:35 PM
lifanov_mail.lifanov.com retitled this revision from to fix regression in xargs -Px and add regression tests.
lifanov_mail.lifanov.com updated this object.
lifanov_mail.lifanov.com edited the test plan for this revision. (Show Details)
Herald added a reviewer: Contributor Reviewers (ports). · View Herald TranscriptAug 7 2015, 1:35 PM
Herald added a subscriber: imp. · View Herald Transcript
lifanov_mail.lifanov.com added reviewers: jbeich, allanjude.Aug 7 2015, 1:36 PM
lifanov_mail.lifanov.com removed a reviewer: Contributor Reviewers (ports).
lifanov_mail.lifanov.com updated this object.
lifanov_mail.lifanov.com edited the test plan for this revision. (Show Details)
jbeich accepted this revision.Aug 7 2015, 2:51 PM
jbeich edited edge metadata.
  • Code looks OK but see inline nit.
  • Tests look OK for rS286289 scope as it didn't touch how processes are spawned.

Note, I'm not a src/ committer to have much weight.

usr.bin/xargs/xargs.c
174 ↗(On Diff #7754)

Unreachable as errstr conditional above already checks valid range:

$ printf 'a\nb\nc\n' | xargs -P -1 echo
xargs: -P -1: too small
This revision is now accepted and ready to land.Aug 7 2015, 2:51 PM
lifanov_mail.lifanov.com updated this revision to Diff 7758.Aug 7 2015, 2:56 PM
lifanov_mail.lifanov.com edited edge metadata.

You are right, maxprocs < 0 is unreachable

This revision now requires review to proceed.Aug 7 2015, 2:56 PM
lifanov_mail.lifanov.com marked an inline comment as done.Aug 7 2015, 2:57 PM

You are right, maxprocs < 0 is unreachable. I updated the diff.

allanjude added reviewers: bapt, marcel.Aug 8 2015, 5:41 PM
allanjude edited edge metadata.Aug 8 2015, 5:48 PM

In the process of being switched from strtoul() to strtonum(), a check against endptr was not removed, causing xargs to segfault

This patch fixes that, and adds a set of regression tests against the -P flag, that were previously missing.

I would like to commit this, to fix the previous commit.

bapt accepted this revision.Aug 8 2015, 5:49 PM
bapt edited edge metadata.
This revision is now accepted and ready to land.Aug 8 2015, 5:49 PM
allanjude commandeered this revision.Aug 8 2015, 6:13 PM
allanjude edited reviewers, added: lifanov_mail.lifanov.com; removed: allanjude.
Closed by commit rS286461: fix regression in xargs -Px (introduced in r286289) and add regression tests (authored by allanjude). · Explain WhyAug 8 2015, 6:37 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
head/
usr.bin/
xargs/
tests/
3 lines
4 lines
1 line
4 lines
5 lines
6 lines

Diff 7785

View Options

head/usr.bin/xargs/tests/Makefile

Show First 20 Lines • Show All 261 Lines • ▼ Show 20 Lines switch (rtype) {
case R_X86_64_COPY: /* none */ case R_X86_64_COPY: /* none */
/* /*
* There shouldn't be copy relocations in kernel * There shouldn't be copy relocations in kernel
* objects. * objects.
*/ */
printf("kldload: unexpected R_COPY relocation\n"); printf("kldload: unexpected R_COPY relocation\n");
return (-1); return (-1);
break;
case R_X86_64_GLOB_DAT: /* S */ case R_X86_64_GLOB_DAT: /* S */
case R_X86_64_JMP_SLOT: /* XXX need addend + offset */ case R_X86_64_JMP_SLOT: /* XXX need addend + offset */
error = lookup(lf, symidx, 1, &addr); error = lookup(lf, symidx, 1, &addr);
if (error != 0) if (error != 0)
return -1; return -1;
if (*where != addr) if (*where != addr)
*where = addr; *where = addr;
break; break;
case R_X86_64_RELATIVE: /* B + A */ case R_X86_64_RELATIVE: /* B + A */
addr = relocbase + addend; addr = elf_relocaddr(lf, relocbase + addend);
val = addr; val = addr;
if (*where != val) if (*where != val)
*where = val; *where = val;
break; break;
case R_X86_64_IRELATIVE: case R_X86_64_IRELATIVE:
addr = relocbase + addend; addr = relocbase + addend;
val = ((Elf64_Addr (*)(void))addr)(); val = ((Elf64_Addr (*)(void))addr)();
▲ Show 20 Lines • Show All 41 Lines • Show Last 20 Lines
View Options

head/usr.bin/xargs/tests/regress.0P1.out

Show First 20 Lines • Show All 261 Lines • ▼ Show 20 Lines switch (rtype) {
case R_X86_64_COPY: /* none */ case R_X86_64_COPY: /* none */
/* /*
* There shouldn't be copy relocations in kernel * There shouldn't be copy relocations in kernel
* objects. * objects.
*/ */
printf("kldload: unexpected R_COPY relocation\n"); printf("kldload: unexpected R_COPY relocation\n");
return (-1); return (-1);
break;
case R_X86_64_GLOB_DAT: /* S */ case R_X86_64_GLOB_DAT: /* S */
case R_X86_64_JMP_SLOT: /* XXX need addend + offset */ case R_X86_64_JMP_SLOT: /* XXX need addend + offset */
error = lookup(lf, symidx, 1, &addr); error = lookup(lf, symidx, 1, &addr);
if (error != 0) if (error != 0)
return -1; return -1;
if (*where != addr) if (*where != addr)
*where = addr; *where = addr;
break; break;
case R_X86_64_RELATIVE: /* B + A */ case R_X86_64_RELATIVE: /* B + A */
addr = relocbase + addend; addr = elf_relocaddr(lf, relocbase + addend);
val = addr; val = addr;
if (*where != val) if (*where != val)
*where = val; *where = val;
break; break;
case R_X86_64_IRELATIVE: case R_X86_64_IRELATIVE:
addr = relocbase + addend; addr = relocbase + addend;
val = ((Elf64_Addr (*)(void))addr)(); val = ((Elf64_Addr (*)(void))addr)();
▲ Show 20 Lines • Show All 41 Lines • Show Last 20 Lines
View Options

head/usr.bin/xargs/tests/regress.P1.out

Show First 20 Lines • Show All 261 Lines • ▼ Show 20 Lines switch (rtype) {
case R_X86_64_COPY: /* none */ case R_X86_64_COPY: /* none */
/* /*
* There shouldn't be copy relocations in kernel * There shouldn't be copy relocations in kernel
* objects. * objects.
*/ */
printf("kldload: unexpected R_COPY relocation\n"); printf("kldload: unexpected R_COPY relocation\n");
return (-1); return (-1);
break;
case R_X86_64_GLOB_DAT: /* S */ case R_X86_64_GLOB_DAT: /* S */
case R_X86_64_JMP_SLOT: /* XXX need addend + offset */ case R_X86_64_JMP_SLOT: /* XXX need addend + offset */
error = lookup(lf, symidx, 1, &addr); error = lookup(lf, symidx, 1, &addr);
if (error != 0) if (error != 0)
return -1; return -1;
if (*where != addr) if (*where != addr)
*where = addr; *where = addr;
break; break;
case R_X86_64_RELATIVE: /* B + A */ case R_X86_64_RELATIVE: /* B + A */
addr = relocbase + addend; addr = elf_relocaddr(lf, relocbase + addend);
val = addr; val = addr;
if (*where != val) if (*where != val)
*where = val; *where = val;
break; break;
case R_X86_64_IRELATIVE: case R_X86_64_IRELATIVE:
addr = relocbase + addend; addr = relocbase + addend;
val = ((Elf64_Addr (*)(void))addr)(); val = ((Elf64_Addr (*)(void))addr)();
▲ Show 20 Lines • Show All 41 Lines • Show Last 20 Lines
View Options

head/usr.bin/xargs/tests/regress.n2P0.out

Show First 20 Lines • Show All 261 Lines • ▼ Show 20 Lines switch (rtype) {
case R_X86_64_COPY: /* none */ case R_X86_64_COPY: /* none */
/* /*
* There shouldn't be copy relocations in kernel * There shouldn't be copy relocations in kernel
* objects. * objects.
*/ */
printf("kldload: unexpected R_COPY relocation\n"); printf("kldload: unexpected R_COPY relocation\n");
return (-1); return (-1);
break;
case R_X86_64_GLOB_DAT: /* S */ case R_X86_64_GLOB_DAT: /* S */
case R_X86_64_JMP_SLOT: /* XXX need addend + offset */ case R_X86_64_JMP_SLOT: /* XXX need addend + offset */
error = lookup(lf, symidx, 1, &addr); error = lookup(lf, symidx, 1, &addr);
if (error != 0) if (error != 0)
return -1; return -1;
if (*where != addr) if (*where != addr)
*where = addr; *where = addr;
break; break;
case R_X86_64_RELATIVE: /* B + A */ case R_X86_64_RELATIVE: /* B + A */
addr = relocbase + addend; addr = elf_relocaddr(lf, relocbase + addend);
val = addr; val = addr;
if (*where != val) if (*where != val)
*where = val; *where = val;
break; break;
case R_X86_64_IRELATIVE: case R_X86_64_IRELATIVE:
addr = relocbase + addend; addr = relocbase + addend;
val = ((Elf64_Addr (*)(void))addr)(); val = ((Elf64_Addr (*)(void))addr)();
▲ Show 20 Lines • Show All 41 Lines • Show Last 20 Lines
View Options

head/usr.bin/xargs/tests/regress.sh

Show First 20 Lines • Show All 261 Lines • ▼ Show 20 Lines switch (rtype) {
case R_X86_64_COPY: /* none */ case R_X86_64_COPY: /* none */
/* /*
* There shouldn't be copy relocations in kernel * There shouldn't be copy relocations in kernel
* objects. * objects.
*/ */
printf("kldload: unexpected R_COPY relocation\n"); printf("kldload: unexpected R_COPY relocation\n");
return (-1); return (-1);
break;
case R_X86_64_GLOB_DAT: /* S */ case R_X86_64_GLOB_DAT: /* S */
case R_X86_64_JMP_SLOT: /* XXX need addend + offset */ case R_X86_64_JMP_SLOT: /* XXX need addend + offset */
error = lookup(lf, symidx, 1, &addr); error = lookup(lf, symidx, 1, &addr);
if (error != 0) if (error != 0)
return -1; return -1;
if (*where != addr) if (*where != addr)
*where = addr; *where = addr;
break; break;
case R_X86_64_RELATIVE: /* B + A */ case R_X86_64_RELATIVE: /* B + A */
addr = relocbase + addend; addr = elf_relocaddr(lf, relocbase + addend);
val = addr; val = addr;
if (*where != val) if (*where != val)
*where = val; *where = val;
break; break;
case R_X86_64_IRELATIVE: case R_X86_64_IRELATIVE:
addr = relocbase + addend; addr = relocbase + addend;
val = ((Elf64_Addr (*)(void))addr)(); val = ((Elf64_Addr (*)(void))addr)();
▲ Show 20 Lines • Show All 41 Lines • Show Last 20 Lines
View Options

head/usr.bin/xargs/xargs.c

Show First 20 Lines • Show All 261 Lines • ▼ Show 20 Lines switch (rtype) {
case R_X86_64_COPY: /* none */ case R_X86_64_COPY: /* none */
/* /*
* There shouldn't be copy relocations in kernel * There shouldn't be copy relocations in kernel
* objects. * objects.
*/ */
printf("kldload: unexpected R_COPY relocation\n"); printf("kldload: unexpected R_COPY relocation\n");
return (-1); return (-1);
break;
case R_X86_64_GLOB_DAT: /* S */ case R_X86_64_GLOB_DAT: /* S */
case R_X86_64_JMP_SLOT: /* XXX need addend + offset */ case R_X86_64_JMP_SLOT: /* XXX need addend + offset */
error = lookup(lf, symidx, 1, &addr); error = lookup(lf, symidx, 1, &addr);
if (error != 0) if (error != 0)
return -1; return -1;
if (*where != addr) if (*where != addr)
*where = addr; *where = addr;
break; break;
case R_X86_64_RELATIVE: /* B + A */ case R_X86_64_RELATIVE: /* B + A */
addr = relocbase + addend; addr = elf_relocaddr(lf, relocbase + addend);
val = addr; val = addr;
if (*where != val) if (*where != val)
*where = val; *where = val;
break; break;
case R_X86_64_IRELATIVE: case R_X86_64_IRELATIVE:
addr = relocbase + addend; addr = relocbase + addend;
val = ((Elf64_Addr (*)(void))addr)(); val = ((Elf64_Addr (*)(void))addr)();
▲ Show 20 Lines • Show All 41 Lines • Show Last 20 Lines