[BusyBox 0001076]: "ip addr del" does not work

Bernhard Fischer rep.nop at aon.at
Sun Nov 5 09:59:15 PST 2006


On Sun, Nov 05, 2006 at 06:24:47PM +0100, Denis Vlasenko wrote:
>> Index: networking/libiproute/ipaddress.c
>> ===================================================================
>> --- networking/libiproute/ipaddress.c   (revision 16438)
>> +++ networking/libiproute/ipaddress.c   (revision 16439)
>> @@ -800,7 +800,7 @@
>>  int do_ipaddr(int argc, char **argv)
>>  {
>>         static const char *const commands[] = {
>> -               "add", "delete", "list", "show", "lst", "flush", 0
>> +               "add", "del", "delete", "list", "show", "lst", "flush", 0
>>         };
>>  
>>         int command_num = 2;
>> [snip rest that introduced another bug, see the link above; The report
>> was automatically sent to the busybox-cvs like any other bugtracker note].
>
>"another bug" is that command_num wasn't updated to 3, right?

right.
>
>> "del" and (i suppose) "lst" are superfluous.
>>   We are supposed to recognize e.g. "a" "ad" as "add", etc.
>>   Adding every possible string strikes me as a very bad idea, size-wise.
>> 
>> 
>> A sane approach is to use
>> - if (strcmp(string_array[i], key) == 0) {
>> + if (strncmp(string_array[i], key, strlen(key)) == 0) {
>> but that did beg the question if we need an indicator to only accept
>> exact matches (like the current code) or not (like the strncmp case).
>> See earlier discussion in this thread.
>
>Do you like attached patch?

Yes, i like it much better, from a short glance (and without having
tested it if "ip route a" throws "ambiguous command" or defaults to
add..).

Thanks for looking into it!

PS: A few vaguely related sidenotes.
- I'd remove "chg", and point people to "change" instead. Not sure how
  many would complain about this, though.
- iplink looks like is may benefit from redoing the way it parses it's
  arguments, too (just as a sidenote).
- iproute.c: Instead of
  bb_error_msg_and_die("need at least destination address");
  use shorter bb_error_msg_and_die("missing destination address");
  Also this: bb_error_msg_and_die("an error :-)"); sounds a bit odd ;)



More information about the busybox mailing list