Request for Review: bin/54151

[Dmitry Morozovsky]

DL> > would you please spend a bit of your time to review
DL> > http://www.freebsd.org/cgi/query-pr.cgi?pr=bin/54151
DL> > [patch to add -i option to arp(8)]?
DL> >
DL> > Thanks in advance; please keep me CC:d as I'm not subscribet to -net.
DL>
DL> The first patch looks ok except for the text of the error message at
DL> source line 157.

Well, it's a piece of old junk: firstly, I used strdup(), and then realized it
isn't necessary for argv. So, these lines possibly should look simply like

@@ -151,6 +154,11 @@
                case 'f' :
                        SETFUNC(F_FILESET);
                        break;
+               case 'i':
+                       rifname = optarg;
+                       if (checkifname(rifname) == 0)
+                               errx(1, "no such interface: %s", rifname);
+                       break;
                case '?':
                default:
                        usage();

DL> I don't think the second patch is necessary.  It might be better to
DL> print a error message if no matching arp entries are found, since each
DL> broadcast interface should at least have its own permanent arp entry.
DL> Checking versus the full interface list doesn't do the correct thing in
DL> any case since non-broadcast interfaces like lo0, serial WAN interfaces,
DL> etc., don't have arp entries.  Should
DL> 	arp -i lo0 -a
DL> be totally silent, or should it print an error message?

Yeah, that was exactly the cause I have separated these two patches. ;-)

Sincerely,
D.Marck                                     [DM5020, MCK-RIPE, DM3-RIPN]
------------------------------------------------------------------------
*** Dmitry Morozovsky --- D.Marck --- Wild Woozle --- marck at rinet.ru ***
------------------------------------------------------------------------