Re: [PATCH] xfsprogs: mkfs: make strict check on -ialign option

[Dave Chinner <david@xxxxxxxxxxxxx>]

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?

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx