On 10-04-20 13:45, Dave Chinner wrote:
> On Mon, Apr 19, 2010 at 05:41:36PM +0800, Wengang Wang wrote:
> > Though it's clearly said in mkfs.xfs man page that for -ialign option only
> > 1 or
> > 0 are valid values, I would like to make a strict check on it in code.
> >
> > If a user specified -ialign=y(but he meant -ialign=1 actually), mkfs treats
> > "y"
> > as "0"(simply by atoi()) thus acts wrongly without complaint. I think we'd
> > better
> > prevent that from happening, so I made the patch. The patch fails the
> > operation
> > on values for -ialign option, like "yes", "no", "y", "n".
> >
> > Signed-off-by: Wengang Wang <wen.gang.wang@xxxxxxxxxx>
> > ---
> > mkfs/xfs_mkfs.c | 7 +++++--
> > 1 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> > index 2d09e36..d7e9eb3 100644
> > --- a/mkfs/xfs_mkfs.c
> > +++ b/mkfs/xfs_mkfs.c
> > @@ -1180,14 +1180,17 @@ main(
> > p = optarg;
> > while (*p != '\0') {
> > char *value;
> > + int len;
> >
> > switch (getsubopt(&p, (constpp)iopts, &value)) {
> > case I_ALIGN:
> > if (!value)
> > value = "1";
> > - iaflag = atoi(value);
> > - if (iaflag < 0 || iaflag > 1)
> > + len = strlen(value);
> > + if (len != 1 || value[0] < '0' ||
> > + value[0] > '1')
> > illegal(value, "i align");
> > + iaflag = value[0] - '0';
>
> Wouldn't this be better changing atoi() to strtol() and then checking
> errno along with the bounds?
Hi Dave,
I tested strtol() with some cases, result is like the following:
<str> -- <result> errno: <errno>
"12" -- 12 errno: 0
"yes" -- 0 errno: 0
"no" -- 0 errno: 0
"y" -- 0 errno: 0
"" -- 0 errno: 0
"0" -- 0 errno: 0
"1" -- 1 errno: 0
It can't tell the original string is "yes", "no", "y", "" or "0" for
returning zero with errno being "OK". That is not the result I want.
regards,
wengang.
|