[Toybox] [Patch] - ifconfig, fixes for IPv6 and SEGFAULT

[Rob Landley]

On 11/22/2013 04:20:07 AM, Ashwini Sharma wrote:
> Hi Rob, list,
> 
> I knwo its too many mails in one day, but thats due to malfunctioned
> mail I had.

No problem. I'm working to catch up. (My multi-week cold has mosty  
cleared up, except for the part that seems to have turned into a mild  
but lingering case of bronchitis.)

> Attacehd is the patch for ifconfig.c,

I actually restarted ifconfig cleanup shortly before you sent this, and  
don't know what parts of this still applies. Let's see...

> fixing issues
> 
> 1. show_help(), only prints help and continues the execution.
>     ifconfig had a SEGFAULT due to this very thing. Modified the
>     the ifconfig_main() to return after showing help for ifconfig.

That still applies. It would be nice if it printed _which_ command  
needed an argument though. (This looks like a job for error_exit().)

I have lib/args.c saying "-c needs 1 argument" so "needs argument"  
isn't new vocabulary to expect people to learn. So I can say "netmask  
needs argument". (I could also say "netmask...?" but I'm not sure  
that's easier for non-english speakers to understand. Hmmm.)

The next question is, if I have a specific message does dumping the  
help text improve matters, or does it just make the error message more  
likely to get lost? Especially since at the moment if I go "ifconfig  
walrus" it just says "ifconfig: walrus: no such device" without dumping  
help text...

The crazy part is that the upstream version behaves both ways,  
"ifconfig blah" doesn't give help text, and "ifconfig netmask" does. I  
suppose the useful difference is:

   $ ifconfig -x
   ifconfig: option `-x' not recognised.
   ifconfig: `--help' gives usage information.
   $ ./ifconfig -x
   ifconfig: -x: No such device

I.E. the toybox version doesn't hint about "--help". Does the fact that  
most toybox commands support --help make up for this? Not sure...

Maybe I should add a new optstring character to exit with help text for  
unknown arguments?

For the moment I'm yanking the show_help(), having error_exit() say  
what the actual problem was, and if we want lib/args.c to say "try  
--help" for an unknown command line option, it should probably do it  
globally.

(I'm open to suggestions on this. I'm a bit under the weather still and  
not really feeling up to making aesthetic design decisions right now.  
Functional, sure, but aesthetic... not so much.)

In the meantime, erring on the side of being terse, because it's unix  
_and_ embedded.

> 2. In __ display_ifconfig() _ there was typo, corrected the filename  
> to
>     "/proc/net/if_inet6".

I caught that one independently.

> 3. in __ show_iface() _ data was read into toybuf. after this _name_
>     pointed inside the toybuf.  This function passed name to __
> display_ifconfig() _
>     function, which again used the toybuf to read data into. Hence
> corrupting the
> _name_. Fixed this by making a copy of name and then freeing it after  
> use.

I think I fixed this already? (I found a couple of these while doing  
the ipv6 cleanup, changed it so display_ifconfig either got iface_name  
or the copy in sl, and neither lives in toybuf.)

Thanks,

Rob
 1387764876.0