Fixes on arp output and parameters validation
ClosedPublic
Actions

Authored by garga on Feb 9 2017, 9:01 AM.

Details

Reviewers

Commits

Summary

'blackhole' and 'reject' are mutually exclusive, replace printf() by errx() when both are selected.
'trail' option is no longer supported since first import of arp from BSD 4.4. XXX message was added 13 years ago in r128192. I believe it's time to remove it.
Use warnx() to print some informative messages instead of printf()
Replace strncmp() by strcmp() when validating parameters and exit when invalid parameter is found

Sponsored by: Rubicon Communications (Netgate)
MFC after: 1 week

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

garga retitled this revision from to Fixes on arp output and parameters validation.

garga updated this object.

garga edited the test plan for this revision. (Show Details)

garga added a reviewer: allanjude.

cem accepted this revision.Feb 9 2017, 5:11 PM

cem edited edge metadata.

This revision is now accepted and ready to land.Feb 9 2017, 5:11 PM

vangyzen edited edge metadata.

vangyzen added inline comments.

usr.sbin/arp/arp.c
322 ↗	(On Diff #24907)	By using `strncmp`, this code accepts parameters like "temperature", "template", and "tempestuous", and treats them like "temp". Likewise for the other cases below. This seems wrong. You didn't break it, obviously, but while you're here...
344 ↗	(On Diff #24907)	I wonder if `blackhole` and `reject` still work after the L2/L3 separation in FreeBSD 8.0. (I think it was 8.0.) Since you're apparently in a cleanup mood... :-)
362 ↗	(On Diff #24907)	`warnx` appends a newline.
398 ↗	(On Diff #24907)	`warnx` appends a newline.
869 ↗	(On Diff #24907)	`warnx` appends a newline. However, this seems like an informative message on the successful path, not a warning, so I think `printf` is more appropriate. In any case, combining the two `printf` calls into one makes sense.

Removed \n from warnx() calls
Back success informative message to printf, but kept a single call
Replace strncmp() by strcmp() on parameters check and exit if wrong parameter is used

This revision now requires review to proceed.Feb 9 2017, 5:44 PM

usr.sbin/arp/arp.c
322 ↗	(On Diff #24907)	Replaced by strcmp() and added else condition to exit for unknown parameters
344 ↗	(On Diff #24907)	I can check it after and submit a new review. Sounds good?
362 ↗	(On Diff #24907)	Fixed
398 ↗	(On Diff #24907)	Fixed
869 ↗	(On Diff #24907)	You are right, back to printf(), but a single one

vangyzen edited edge metadata.

vangyzen added inline comments.

usr.sbin/arp/arp.c
489 ↗	(On Diff #24933)	Here's another extra newline that I missed the first time.
344 ↗	(On Diff #24907)	Sure. It's a logically separate change, so it should be a separate commit.