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

[Bernhard Fischer]

On Sun, Nov 05, 2006 at 01:57:26AM +0100, Denis Vlasenko wrote:
>> >> there's a recommended rule that functions be given names so that, when
>> >> used in their natural context, they make intuitive sense.  for
>> >> instance,
>> >> 
>> >>   if (compare_string_array(...)) {
>> >> 
>> >> doesn't really suggest what's being returned, but
>> >> 
>> >>   if (string_array_contains_string(...)) {
>> >> 
>> >> should be immediately obvious.  or something like that.
>> >
>> >compare_string_array() returns index of string, not just 1/0.
>> >index_in_str_array()?
>> 
>> Given that my copy that reads strncmp() in there still lives on locally,
>> i take it that this isn't fixed yet?
>
>Sorry, I was working on making ps vaguely resemble POSIX ps lately...
>now it is good time to actually test how it works.
>(I use ps in my shutdown scripts)
>
>
>What is the problem here? In svn I see the following:

http://busybox.net/bugs/view.php?id=1076
>
>int compare_string_array(const char * const string_array[], const char *key)
>{
>        int i;
>
>        for (i = 0; string_array[i] != 0; i++) {
>                if (strcmp(string_array[i], key) == 0) {
>                        return i;
>                }
>        }
>        return -i;
>}
>
>Apart from name in need of changing to index_in_str_array(), I don't
>see any problem here. Please give more details.

The fix that went in as r16439 is wrong:
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].

"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.