[Toybox] [Patch - New Toy] Add traceroute

[Ashwini Sharma]

On Sun, Sep 15, 2013 at 1:24 PM, Isaac <ibid.ag at gmail.com> wrote:

> On Sun, Sep 15, 2013 at 11:59:42AM +0900, Ashwini Sharma wrote:
> > On Fri, Sep 13, 2013 at 11:31 PM, Isaac <ibid.ag at gmail.com> wrote:
> > > I note that it appears to revert one of the recent changes to
> > > lib/pending.c.
> > >
> > > (Would you mind sending new toys as just the *.c file that goes in
> toys/*/
> > > when it doesn't need to touch lib/?)
>
> > my patch on lib/pending.c is only modifying  get_int_value() function.
> > Checking '-' and leading white spaces in the string.
> >
> > I changed this
> > -  if (errno || numstr == ptr || *ptr || rvalue < lowrange || rvalue >
> > highrange)
> > -    perror_exit("bad number '%s'", numstr);
> >
> > to
> >
> > +  if(errno || *ptr) perror_exit("invalid number '%s'", numstr);
> > +  if(rvalue < lowrange || rvalue > highrange)
> > +    error_exit("out of range '%s'", numstr);
> >
> > for having a better error message.
> >
>
> See this patch in commit 1066 (manually munged to indicate intent...):
>
>    unsigned long rvalue = 0;
>    char *ptr;
> -  if(*numstr == '-' || *numstr == '+' || isspace(*numstr))
> perror_exit("invalid number '%s'", numstr);
> +
> +  if (!isdigit(*numstr)) perror_exit("bad number '%s'", numstr);
>    errno = 0;
>    rvalue = strtoul(numstr, &ptr, 10);
> -  if(errno || numstr == ptr) perror_exit("invalid number '%s'", numstr);
> -   if(*ptr) perror_exit("invalid number '%s'", numstr);
> -   if(rvalue >= lowrange && rvalue <= highrange) return rvalue;
> -   else {
> -         perror_exit("invalid number '%s'", numstr);
> -         return rvalue; //Not reachable; to avoid waring message.
> -   }
> +  if (errno || numstr == ptr || *ptr || rvalue < lowrange || rvalue >
> highrange)
> +    perror_exit("bad number '%s'", numstr);
> +  return rvalue;
>  }
>

Aah!! when I generated patch, these chages were not reflecting in hg.
Better not to apply my lib/pending.c patch


>
> Rob's mentioned a few times that simpler messages are preferred;
> http://landley.net/toybox/cleanup.html refers to this message:
>
> http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000850.html
>
> And that's part of the rationale listed in the change.
>
> (however, I don't see how get_int_value can handle negative numbers;
> rvalue and lowrange are unsigned longs...)
>
> If I were the one doing it, I might use "bad number" and "not in range",
> or maybe even for the latter (if it works! Haven't tested)
> perror_exit("'%s' too '%s'", numstr, rvalue < lowrange ? "low" : "high");
> But that last might be a little more clever and less clear...
> In general, s/invalid/bad/.
>
> > IPv6 is in fact important. Working on it, but was confused whether to
> have
> > the IPv6 support
> > in the same traceroute.c or separate. As far as I see, there will be lot
> > many if/else checks for IPv4/IPv6 cases.
> > Like for the incoming packet parsing.
>
> Same file. A separate IPv6 command is not desireable (Rob recenty
> mentioned this somewhere--I think in the discussion of Strake's
> mount or sed), though I could see a <name>6 OLDTOY() for those who
> are accustomed to the two binaries...
>
> > Any suggestions are welcome.
> >
> > regards,
> > Ashwini
>
> Thanks,
> Isaac Dunham
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20130916/efb7abed/attachment-0005.htm>