<br><br>
<div class="gmail_quote">On Sun, Sep 15, 2013 at 1:24 PM, Isaac <span dir="ltr"><<a href="mailto:ibid.ag@gmail.com" target="_blank">ibid.ag@gmail.com</a>></span> wrote:<br>
<blockquote style="BORDER-LEFT:#ccc 1px solid;MARGIN:0px 0px 0px 0.8ex;PADDING-LEFT:1ex" class="gmail_quote">
<div class="im">On Sun, Sep 15, 2013 at 11:59:42AM +0900, Ashwini Sharma wrote:<br>> On Fri, Sep 13, 2013 at 11:31 PM, Isaac <<a href="mailto:ibid.ag@gmail.com">ibid.ag@gmail.com</a>> wrote:<br></div>
<div class="im">> > I note that it appears to revert one of the recent changes to<br>> > lib/pending.c.<br>> ><br>> > (Would you mind sending new toys as just the *.c file that goes in toys/*/<br>
> > when it doesn't need to touch lib/?)<br><br></div>
<div class="im">> my patch on lib/pending.c is only modifying  get_int_value() function.<br>> Checking '-' and leading white spaces in the string.<br>><br>> I changed this<br>> -  if (errno || numstr == ptr || *ptr || rvalue < lowrange || rvalue ><br>
> highrange)<br>> -    perror_exit("bad number '%s'", numstr);<br>><br>> to<br>><br>> +  if(errno || *ptr) perror_exit("invalid number '%s'", numstr);<br>> +  if(rvalue < lowrange || rvalue > highrange)<br>
> +    error_exit("out of range '%s'", numstr);<br>><br>> for having a better error message.<br>><br><br></div>See this patch in commit 1066 (manually munged to indicate intent...):<br><br>   unsigned long rvalue = 0;<br>
   char *ptr;<br>-  if(*numstr == '-' || *numstr == '+' || isspace(*numstr)) perror_exit("invalid number '%s'", numstr);<br>+<br>+  if (!isdigit(*numstr)) perror_exit("bad number '%s'", numstr);<br>
   errno = 0;<br>   rvalue = strtoul(numstr, &ptr, 10);<br>-  if(errno || numstr == ptr) perror_exit("invalid number '%s'", numstr);<br>-   if(*ptr) perror_exit("invalid number '%s'", numstr);<br>
-   if(rvalue >= lowrange && rvalue <= highrange) return rvalue;<br>-   else {<br>
<div class="im">-         perror_exit("invalid number '%s'", numstr);<br></div>-         return rvalue; //Not reachable; to avoid waring message.<br>-   }<br>
<div class="im">+  if (errno || numstr == ptr || *ptr || rvalue < lowrange || rvalue > highrange)<br>+    perror_exit("bad number '%s'", numstr);<br></div>+  return rvalue;<br> }<br></blockquote>
<div> </div>
<div>Aah!! when I generated patch, these chages were not reflecting in hg. Better not to apply my lib/pending.c patch</div>
<div> </div>
<blockquote style="BORDER-LEFT:#ccc 1px solid;MARGIN:0px 0px 0px 0.8ex;PADDING-LEFT:1ex" class="gmail_quote"><br>Rob's mentioned a few times that simpler messages are preferred;<br><a href="http://landley.net/toybox/cleanup.html" target="_blank">http://landley.net/toybox/cleanup.html</a> refers to this message:<br>
<a href="http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000850.html" target="_blank">http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000850.html</a><br><br>And that's part of the rationale listed in the change.<br>
<br>(however, I don't see how get_int_value can handle negative numbers;<br>rvalue and lowrange are unsigned longs...)<br><br>If I were the one doing it, I might use "bad number" and "not in range",<br>
or maybe even for the latter (if it works! Haven't tested)<br>perror_exit("'%s' too '%s'", numstr, rvalue < lowrange ? "low" : "high");<br>But that last might be a little more clever and less clear...<br>
In general, s/invalid/bad/.<br>
<div class="im"><br>> IPv6 is in fact important. Working on it, but was confused whether to have<br>> the IPv6 support<br>> in the same traceroute.c or separate. As far as I see, there will be lot<br>> many if/else checks for IPv4/IPv6 cases.<br>
> Like for the incoming packet parsing.<br><br></div>Same file. A separate IPv6 command is not desireable (Rob recenty<br>mentioned this somewhere--I think in the discussion of Strake's<br>mount or sed), though I could see a <name>6 OLDTOY() for those who<br>
are accustomed to the two binaries...<br>
<div class="im"><br>> Any suggestions are welcome.<br>><br>> regards,<br>> Ashwini<br><br></div>Thanks,<br>Isaac Dunham<br></blockquote></div><br>