On 14 January 2011 c. 03:00:30 Ted Unangst wrote:
> On Thu, Jan 13, 2011 at 8:48 AM, Vadim Zhukov <[email protected]> wrote:
> > case 'S':
> > - sectorsize = strtonum(optarg, 1, INT_MAX,
&errstr);
> > - if (errstr)
> > - fatal("sector size is %s: %s", errstr,
optarg);
> > + if (scan_scaled(optarg, §orsize) == -1 ||
> > + sectorsize <= 0 || sectorsize > INT_MAX)
> > + fatal("sector size: %s: %s",
strerror(errno),
> > + optarg);
>
> I don't think we need this. There aren't many choices, and all of
> them are small numbers people know how to type.
OK. I just tried to save as much old behaviour as I can. :)
> > case 's':
> > + /*
> > + * We need to save scaled and unscaled value
separately
> > + * because unscaled is not representing bytes.
> > + */
> > + fssize_scaled = -1; /* in case of multiple -s
*/
> > fssize = strtonum(optarg, 1, LLONG_MAX, &errstr);
> > - if (errstr)
> > - fatal("file system size is %s: %s",
> > - errstr, optarg);
> > + if (!errstr)
> > + break;
> > + if (strcmp(errstr, "invalid") ||
> > + scan_scaled(optarg, &fssize_scaled) == -1 ||
> > + fssize_scaled <= 0)
> > + fatal("file system size is %s: %s",
errstr, optarg);
>
> This is definite strtonum abuse. I'd prefer to see something that
> tries a little harder to call the right function.
Me too.
> Also, things get a little slippery because you're leaving this block
> of code with only one variable set properly. Get the right value into
> fssize before leaving the case statement.
Fixed version will be in a few minutes in another mail.
--
Best wishes,
Vadim Zhukov
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?
