Re: [Toybox] - Add route

2013-09-11 Thread Felix Janda
Ashwini Sharma wrote:
> On Wed, Sep 11, 2013 at 5:30 AM, Felix Janda  wrote:
> 
> > Ashwini Sharma wrote:
[...]
> > > Didn't use show_help(), as this doesn't exit whereas an exit is intended
> > > for route.
> > > may be show_help() can be updated to do that.
> >
> > For this there is toys.exithelp. Set it to one and then call error_exit().
> >
> 
> Yup, show_route_help, already does that. It was to be called many times,
> hence made a function
> show_route_help().

Oh, sorry for not looking at your patch. (Could you maybe get your MUA to 
produce
text/x-patch attachments?)

You're right it's called quite often. It seems that the command is doing
argparsing in several places so that you can't just set toys.exithelp in the
beginning and reset it when you're done with the argparsing.

Felix
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net


Re: [Toybox] - Add route

2013-09-10 Thread Ashwini Sharma
On Wed, Sep 11, 2013 at 5:30 AM, Felix Janda  wrote:

> Ashwini Sharma wrote:
> > On Sat, Sep 7, 2013 at 11:47 PM, Isaac  wrote:
> >
> > > On Mon, Sep 02, 2013 at 11:15:14AM +0900, Ashwini Sharma wrote:
> > > > HI Rob & list,
> > > >
> > > >   Attached is the patch for _route_ command. It does display, add
> and del
> > > > functions for routing tables.
> > > > Have a look at it.
> > > >
> > > > regards,
> > > > Ashwini Kumar
> > >
> > > OK, I took a quick look through it.
> > > First, thanks for doing route.
> > >
> > > I note that show_route_help is identical to show_help() apart from the
> > > error message.  That's probably not needed.
> > >
> >
> > Didn't use show_help(), as this doesn't exit whereas an exit is intended
> > for route.
> > may be show_help() can be updated to do that.
>
> For this there is toys.exithelp. Set it to one and then call error_exit().
>

Yup, show_route_help, already does that. It was to be called many times,
hence made a function
show_route_help().


>
> Felix
>
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net


Re: [Toybox] - Add route

2013-09-10 Thread Felix Janda
Ashwini Sharma wrote:
> On Sat, Sep 7, 2013 at 11:47 PM, Isaac  wrote:
> 
> > On Mon, Sep 02, 2013 at 11:15:14AM +0900, Ashwini Sharma wrote:
> > > HI Rob & list,
> > >
> > >   Attached is the patch for _route_ command. It does display, add and del
> > > functions for routing tables.
> > > Have a look at it.
> > >
> > > regards,
> > > Ashwini Kumar
> >
> > OK, I took a quick look through it.
> > First, thanks for doing route.
> >
> > I note that show_route_help is identical to show_help() apart from the
> > error message.  That's probably not needed.
> >
> 
> Didn't use show_help(), as this doesn't exit whereas an exit is intended
> for route.
> may be show_help() can be updated to do that.

For this there is toys.exithelp. Set it to one and then call error_exit().

Felix
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net


Re: [Toybox] - Add route

2013-09-07 Thread Ashwini Sharma
On Sat, Sep 7, 2013 at 11:47 PM, Isaac  wrote:

> On Mon, Sep 02, 2013 at 11:15:14AM +0900, Ashwini Sharma wrote:
> > HI Rob & list,
> >
> >   Attached is the patch for _route_ command. It does display, add and del
> > functions for routing tables.
> > Have a look at it.
> >
> > regards,
> > Ashwini Kumar
>
> OK, I took a quick look through it.
> First, thanks for doing route.
>
> I note that show_route_help is identical to show_help() apart from the
> error message.  That's probably not needed.
>

Didn't use show_help(), as this doesn't exit whereas an exit is intended
for route.
may be show_help() can be updated to do that.


> Now...
> Hmm, there's an else if (!strcmp) ladder.
> It could be reduced to a switch if we take
> *argv[0]<<16 + *argv[1]<<8 + *argv[2]
>

didn't get your logic here, may be a little more detail can help.


>
> And the help could use a little modification to make it more commonly
> understandable:
> route [-ne] [-A inet | inet6] [add target | del target]
> ...
> -A inet(6)  Select...
> (note one space between the option and its parameter,
> but two between the parameter and the explanation)
>

Yes, help message can be updated to the one suggested.


> The rest I don't understand well enough to comment on.
>
> Thanks,
> Isaac Dunham
>

Thanks,
Ashwini Kumar
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net


Re: [Toybox] - Add route

2013-09-07 Thread Isaac
On Mon, Sep 02, 2013 at 11:15:14AM +0900, Ashwini Sharma wrote:
> HI Rob & list,
> 
>   Attached is the patch for _route_ command. It does display, add and del
> functions for routing tables.
> Have a look at it.
> 
> regards,
> Ashwini Kumar

OK, I took a quick look through it.
First, thanks for doing route.

I note that show_route_help is identical to show_help() apart from the 
error message.  That's probably not needed.
Now...
Hmm, there's an else if (!strcmp) ladder.
It could be reduced to a switch if we take 
*argv[0]<<16 + *argv[1]<<8 + *argv[2]

And the help could use a little modification to make it more commonly 
understandable:
route [-ne] [-A inet | inet6] [add target | del target]
...
-A inet(6)  Select...
(note one space between the option and its parameter, 
but two between the parameter and the explanation)

The rest I don't understand well enough to comment on.

Thanks,
Isaac Dunham
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net


[Toybox] - Add route

2013-09-01 Thread Ashwini Sharma
HI Rob & list,

  Attached is the patch for _route_ command. It does display, add and del
functions for routing tables.
Have a look at it.

regards,
Ashwini Kumar


route.patch
Description: Binary data
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net