<div dir="ltr"><div class="gmail_default" style="font-family:arial,helvetica,sans-serif"><span style="font-family:arial,sans-serif">On Thu, Apr 7, 2016 at 9:02 PM, Eric Sandeen </span><span dir="ltr" style="font-family:arial,sans-serif"><<a href="mailto:sandeen@sandeen.net" target="_blank">sandeen@sandeen.net</a>></span><span style="font-family:arial,sans-serif"> wrote:</span><br></div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<br>
On 3/24/16 6:15 AM, <a href="mailto:jtulak@redhat.com">jtulak@redhat.com</a> wrote:<br>
> From: Dave Chinner <<a href="mailto:dchinner@redhat.com">dchinner@redhat.com</a>><br>
><br>
> Now that getnum_checked can handle min/max checking, use this for<br>
> all parameters that take straight numbers and don't require unit<br>
> conversions.<br>
><br>
> Signed-off-by: Dave Chinner <<a href="mailto:dchinner@redhat.com">dchinner@redhat.com</a>><br>
> Signed-off-by: Jan Tulak <<a href="mailto:jtulak@redhat.com">jtulak@redhat.com</a>><br>
<br>
</span>Looks ok.<br>
<br>
Signed-off-by: Eric Sandeen <<a href="mailto:sandeen@redhat.com">sandeen@redhat.com</a>><br></blockquote><div><br></div><div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;display:inline">​Or Reviewed-by? :-)</div></div><div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;display:inline"><br></div></div><div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">​Thanks,</div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">Jan​</div><br></div><div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;display:inline">​</div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
> ---<br>
>  include/xfs_multidisk.h |   5 +-<br>
>  mkfs/xfs_mkfs.c         | 148 ++++++++++++++++++++++++------------------------<br>
>  2 files changed, 76 insertions(+), 77 deletions(-)<br>
><br>
> diff --git a/include/xfs_multidisk.h b/include/xfs_multidisk.h<br>
> index af35100..8e81d90 100644<br>
> --- a/include/xfs_multidisk.h<br>
> +++ b/include/xfs_multidisk.h<br>
> @@ -42,8 +42,9 @@<br>
><br>
>  #define XFS_AG_BYTES(bblog)  ((long long)BBSIZE << (bblog))<br>
>  #define      XFS_AG_MIN_BYTES        ((XFS_AG_BYTES(15)))    /* 16 MB */<br>
> -#define XFS_AG_MIN_BLOCKS(blog)      ((XFS_AG_BYTES(15)) >> (blog))<br>
> -#define XFS_AG_MAX_BLOCKS(blog)      ((XFS_AG_BYTES(31) - 1) >> (blog))<br>
> +#define      XFS_AG_MAX_BYTES        ((XFS_AG_BYTES(31)))    /* 1 TB */<br>
> +#define XFS_AG_MIN_BLOCKS(blog)      (XFS_AG_MIN_BYTES >> (blog))<br>
> +#define XFS_AG_MAX_BLOCKS(blog)      ((XFS_AG_MAX_BYTES - 1) >> (blog))<br>
><br>
>  #define XFS_MAX_AGNUMBER     ((xfs_agnumber_t)(NULLAGNUMBER - 1))<br>
><br>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c<br>
> index 0e2cfac..021d682 100644<br>
> --- a/mkfs/xfs_mkfs.c<br>
> +++ b/mkfs/xfs_mkfs.c<br>
> @@ -163,6 +163,8 @@ struct opt_params dopts = {<br>
>       },<br>
>       .subopt_params = {<br>
>               { .index = D_AGCOUNT,<br>
> +               .minval = 1,<br>
> +               .maxval = XFS_MAX_AGNUMBER,<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = D_FILE,<br>
> @@ -177,18 +179,26 @@ struct opt_params dopts = {<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = D_SUNIT,<br>
> +               .minval = 0,<br>
> +               .maxval = UINT_MAX,<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = D_SWIDTH,<br>
> +               .minval = 0,<br>
> +               .maxval = UINT_MAX,<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = D_AGSIZE,<br>
> +               .minval = XFS_AG_MIN_BYTES,<br>
> +               .maxval = XFS_AG_MAX_BYTES,<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = D_SU,<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = D_SW,<br>
> +               .minval = 0,<br>
> +               .maxval = UINT_MAX,<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = D_SECTLOG,<br>
> @@ -207,12 +217,18 @@ struct opt_params dopts = {<br>
>                 .defaultval = 1,<br>
>               },<br>
>               { .index = D_RTINHERIT,<br>
> -               .defaultval = SUBOPT_NEEDS_VAL,<br>
> +               .minval = 1,<br>
> +               .maxval = 1,<br>
> +               .defaultval = 1,<br>
>               },<br>
>               { .index = D_PROJINHERIT,<br>
> +               .minval = 0,<br>
> +               .maxval = UINT_MAX,<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = D_EXTSZINHERIT,<br>
> +               .minval = 0,<br>
> +               .maxval = UINT_MAX,<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>       },<br>
> @@ -252,15 +268,23 @@ struct opt_params iopts = {<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = I_MAXPCT,<br>
> +               .minval = 0,<br>
> +               .maxval = 100,<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = I_PERBLOCK,<br>
> +               .minval = XFS_MIN_INODE_PERBLOCK,<br>
> +               .maxval = XFS_MAX_BLOCKSIZE / XFS_DINODE_MIN_SIZE,<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = I_SIZE,<br>
> +               .minval = XFS_DINODE_MIN_SIZE,<br>
> +               .maxval = XFS_DINODE_MAX_SIZE,<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = I_ATTR,<br>
> +               .minval = 0,<br>
> +               .maxval = 2,<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = I_PROJID32BIT,<br>
> @@ -307,6 +331,8 @@ struct opt_params lopts = {<br>
>       },<br>
>       .subopt_params = {<br>
>               { .index = L_AGNUM,<br>
> +               .minval = 0,<br>
> +               .maxval = UINT_MAX,<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = L_INTERNAL,<br>
> @@ -318,9 +344,13 @@ struct opt_params lopts = {<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = L_VERSION,<br>
> +               .minval = 1,<br>
> +               .maxval = 2,<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = L_SUNIT,<br>
> +               .minval = BTOBB(XLOG_MIN_RECORD_BSIZE),<br>
> +               .maxval = BTOBB(XLOG_MAX_RECORD_BSIZE),<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = L_SU,<br>
> @@ -380,6 +410,8 @@ struct opt_params nopts = {<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = N_VERSION,<br>
> +               .minval = 2,<br>
> +               .maxval = 2,<br>
>                 .defaultval = SUBOPT_NEEDS_VAL,<br>
>               },<br>
>               { .index = N_FTYPE,<br>
> @@ -1560,13 +1592,11 @@ main(<br>
>                               switch (getsubopt(&p, (constpp)subopts,<br>
>                                                 &value)) {<br>
>                               case D_AGCOUNT:<br>
> -                                     if (!value || *value == '\0')<br>
> -                                             reqval('d', subopts, D_AGCOUNT);<br>
>                                       if (daflag)<br>
>                                               respec('d', subopts, D_AGCOUNT);<br>
> -                                     agcount = getnum(value, 0, 0, false);<br>
> -                                     if ((__int64_t)agcount <= 0)<br>
> -                                             illegal(value, "d agcount");<br>
> +<br>
> +                                     agcount = getnum_checked(value, &dopts,<br>
> +                                                              D_AGCOUNT);<br>
>                                       daflag = 1;<br>
>                                       break;<br>
>                               case D_AGSIZE:<br>
> @@ -1601,28 +1631,22 @@ main(<br>
>                                       dsize = value;<br>
>                                       break;<br>
>                               case D_SUNIT:<br>
> -                                     if (!value || *value == '\0')<br>
> -                                             reqval('d', subopts, D_SUNIT);<br>
>                                       if (dsunit)<br>
>                                               respec('d', subopts, D_SUNIT);<br>
>                                       if (nodsflag)<br>
>                                               conflict('d', subopts, D_NOALIGN,<br>
>                                                        D_SUNIT);<br>
> -                                     dsunit = getnum(value, 0, 0, false);<br>
> -                                     if (dsunit < 0)<br>
> -                                             illegal(value, "d sunit");<br>
> +                                     dsunit = getnum_checked(value, &dopts,<br>
> +                                                              D_SUNIT);<br>
>                                       break;<br>
>                               case D_SWIDTH:<br>
> -                                     if (!value || *value == '\0')<br>
> -                                             reqval('d', subopts, D_SWIDTH);<br>
>                                       if (dswidth)<br>
>                                               respec('d', subopts, D_SWIDTH);<br>
>                                       if (nodsflag)<br>
>                                               conflict('d', subopts, D_NOALIGN,<br>
>                                                        D_SWIDTH);<br>
> -                                     dswidth = getnum(value, 0, 0, false);<br>
> -                                     if (dswidth < 0)<br>
> -                                             illegal(value, "d swidth");<br>
> +                                     dswidth = getnum_checked(value, &dopts,<br>
> +                                                              D_SWIDTH);<br>
>                                       break;<br>
>                               case D_SU:<br>
>                                       if (!value || *value == '\0')<br>
> @@ -1638,16 +1662,13 @@ main(<br>
>                                               illegal(value, "d su");<br>
>                                       break;<br>
>                               case D_SW:<br>
> -                                     if (!value || *value == '\0')<br>
> -                                             reqval('d', subopts, D_SW);<br>
>                                       if (dsw)<br>
>                                               respec('d', subopts, D_SW);<br>
>                                       if (nodsflag)<br>
>                                               conflict('d', subopts, D_NOALIGN,<br>
>                                                        D_SW);<br>
> -                                     dsw = getnum(value, 0, 0, false);<br>
> -                                     if (dsw < 0)<br>
> -                                             illegal(value, "d sw");<br>
> +                                     dsw = getnum_checked(value, &dopts,<br>
> +                                                              D_SW);<br>
>                                       break;<br>
>                               case D_NOALIGN:<br>
>                                       nodsflag = getnum_checked(value,<br>
> @@ -1696,21 +1717,22 @@ main(<br>
>                                       ssflag = 1;<br>
>                                       break;<br>
>                               case D_RTINHERIT:<br>
> -                                     fsx.fsx_xflags |= \<br>
> -                                             XFS_DIFLAG_RTINHERIT;<br>
> +                                     c = getnum_checked(value, &dopts,<br>
> +                                                        D_RTINHERIT);<br>
> +                                     if (c)<br>
> +                                             fsx.fsx_xflags |=<br>
> +                                                     XFS_DIFLAG_RTINHERIT;<br>
>                                       break;<br>
>                               case D_PROJINHERIT:<br>
> -                                     if (!value || *value == '\0')<br>
> -                                             reqval('d', subopts, D_PROJINHERIT);<br>
> -                                     fsx.fsx_projid = atoi(value);<br>
> -                                     fsx.fsx_xflags |= \<br>
> +                                     fsx.fsx_projid = getnum_checked(value,<br>
> +                                                     &dopts, D_PROJINHERIT);<br>
> +                                     fsx.fsx_xflags |=<br>
>                                               XFS_DIFLAG_PROJINHERIT;<br>
>                                       break;<br>
>                               case D_EXTSZINHERIT:<br>
> -                                     if (!value || *value == '\0')<br>
> -                                             reqval('d', subopts, D_EXTSZINHERIT);<br>
> -                                     fsx.fsx_extsize = atoi(value);<br>
> -                                     fsx.fsx_xflags |= \<br>
> +                                     fsx.fsx_extsize = getnum_checked(value,<br>
> +                                                     &dopts, D_EXTSZINHERIT);<br>
> +                                     fsx.fsx_xflags |=<br>
>                                               XFS_DIFLAG_EXTSZINHERIT;<br>
>                                       break;<br>
>                               default:<br>
> @@ -1745,18 +1767,13 @@ main(<br>
>                                       ilflag = 1;<br>
>                                       break;<br>
>                               case I_MAXPCT:<br>
> -                                     if (!value || *value == '\0')<br>
> -                                             reqval('i', subopts, I_MAXPCT);<br>
>                                       if (imflag)<br>
>                                               respec('i', subopts, I_MAXPCT);<br>
> -                                     imaxpct = getnum(value, 0, 0, false);<br>
> -                                     if (imaxpct < 0 || imaxpct > 100)<br>
> -                                             illegal(value, "i maxpct");<br>
> +                                     imaxpct = getnum_checked(<br>
> +                                                     value, &iopts, I_MAXPCT);<br>
>                                       imflag = 1;<br>
>                                       break;<br>
>                               case I_PERBLOCK:<br>
> -                                     if (!value || *value == '\0')<br>
> -                                             reqval('i', subopts, I_PERBLOCK);<br>
>                                       if (ilflag)<br>
>                                               conflict('i', subopts, I_LOG,<br>
>                                                        I_PERBLOCK);<br>
> @@ -1765,16 +1782,13 @@ main(<br>
>                                       if (isflag)<br>
>                                               conflict('i', subopts, I_SIZE,<br>
>                                                        I_PERBLOCK);<br>
> -                                     inopblock = getnum(value, 0, 0, false);<br>
> -                                     if (inopblock <<br>
> -                                             XFS_MIN_INODE_PERBLOCK ||<br>
> -                                         !ispow2(inopblock))<br>
> +                                     inopblock = getnum_checked(value, &iopts,<br>
> +                                                                I_PERBLOCK);<br>
> +                                     if (!ispow2(inopblock))<br>
>                                               illegal(value, "i perblock");<br>
>                                       ipflag = 1;<br>
>                                       break;<br>
>                               case I_SIZE:<br>
> -                                     if (!value || *value == '\0')<br>
> -                                             reqval('i', subopts, I_SIZE);<br>
>                                       if (ilflag)<br>
>                                               conflict('i', subopts, I_LOG,<br>
>                                                        I_SIZE);<br>
> @@ -1783,19 +1797,16 @@ main(<br>
>                                                        I_SIZE);<br>
>                                       if (isflag)<br>
>                                               respec('i', subopts, I_SIZE);<br>
> -                                     isize = getnum(value, 0, 0, true);<br>
> -                                     if (isize <= 0 || !ispow2(isize))<br>
> +                                     isize = getnum_checked(value, &iopts,<br>
> +                                                            I_SIZE);<br>
> +                                     if (!ispow2(isize))<br>
>                                               illegal(value, "i size");<br>
>                                       inodelog = libxfs_highbit32(isize);<br>
>                                       isflag = 1;<br>
>                                       break;<br>
>                               case I_ATTR:<br>
> -                                     if (!value || *value == '\0')<br>
> -                                             reqval('i', subopts, I_ATTR);<br>
> -                                     c = getnum(value, 0, 0, false);<br>
> -                                     if (c < 0 || c > 2)<br>
> -                                             illegal(value, "i attr");<br>
> -                                     sb_feat.attr_version = c;<br>
> +                                     sb_feat.attr_version = getnum_checked(<br>
> +                                                     value, &iopts, I_ATTR);<br>
>                                       break;<br>
>                               case I_PROJID32BIT:<br>
>                                       sb_feat.projid16bit =<br>
> @@ -1817,21 +1828,16 @@ main(<br>
>                       while (*p != '\0') {<br>
>                               char    **subopts = (char **)lopts.subopts;<br>
>                               char    *value;<br>
> -                             long long tmp_num;<br>
><br>
>                               switch (getsubopt(&p, (constpp)subopts,<br>
>                                                 &value)) {<br>
>                               case L_AGNUM:<br>
> -                                     if (!value || *value == '\0')<br>
> -                                             reqval('l', subopts, L_AGNUM);<br>
>                                       if (laflag)<br>
>                                               respec('l', subopts, L_AGNUM);<br>
>                                       if (ldflag)<br>
>                                               conflict('l', subopts, L_AGNUM, L_DEV);<br>
> -                                     tmp_num = getnum(value, 0, 0, false);<br>
> -                                     if (tmp_num < 0)<br>
> -                                             illegal(value, "l agno");<br>
> -                                     logagno = (xfs_agnumber_t)tmp_num;<br>
> +                                     logagno = getnum_checked(value, &lopts,<br>
> +                                                              L_AGNUM);<br>
>                                       laflag = 1;<br>
>                                       break;<br>
>                               case L_FILE:<br>
> @@ -1868,13 +1874,10 @@ main(<br>
>                                       lsuflag = 1;<br>
>                                       break;<br>
>                               case L_SUNIT:<br>
> -                                     if (!value || *value == '\0')<br>
> -                                             reqval('l', subopts, L_SUNIT);<br>
>                                       if (lsunit)<br>
>                                               respec('l', subopts, L_SUNIT);<br>
> -                                     lsunit = getnum(value, 0, 0, false);<br>
> -                                     if (lsunit < 0)<br>
> -                                             illegal(value, "l sunit");<br>
> +                                     lsunit = getnum_checked(value, &lopts,<br>
> +                                                              L_SUNIT);<br>
>                                       lsunitflag = 1;<br>
>                                       break;<br>
>                               case L_NAME:<br>
> @@ -1893,14 +1896,10 @@ main(<br>
>                                       xi.logname = value;<br>
>                                       break;<br>
>                               case L_VERSION:<br>
> -                                     if (!value || *value == '\0')<br>
> -                                             reqval('l', subopts, L_VERSION);<br>
>                                       if (lvflag)<br>
>                                               respec('l', subopts, L_VERSION);<br>
> -                                     c = getnum(value, 0, 0, false);<br>
> -                                     if (c < 1 || c > 2)<br>
> -                                             illegal(value, "l version");<br>
> -                                     sb_feat.log_version = c;<br>
> +                                     sb_feat.log_version = getnum_checked(<br>
> +                                             value, &lopts, L_VERSION);<br>
>                                       lvflag = 1;<br>
>                                       break;<br>
>                               case L_SIZE:<br>
> @@ -2035,11 +2034,10 @@ _("cannot specify both -m crc=1 and -n ftype\n"));<br>
>                                               /* ASCII CI mode */<br>
>                                               sb_feat.nci = true;<br>
>                                       } else {<br>
> -                                             c = getnum(value, 0, 0, false);<br>
> -                                             if (c != 2)<br>
> -                                                     illegal(value,<br>
> -                                                             "n version");<br>
> -                                             sb_feat.dir_version = c;<br>
> +                                             sb_feat.dir_version =<br>
> +                                                     getnum_checked(value,<br>
> +                                                             &nopts,<br>
> +                                                             N_VERSION);<br>
>                                       }<br>
>                                       nvflag = 1;<br>
>                                       break;<br>
><br>
<br>
</div></div><div class="HOEnZb"><div class="h5">_______________________________________________<br>
xfs mailing list<br>
<a href="mailto:xfs@oss.sgi.com">xfs@oss.sgi.com</a><br>
<a href="http://oss.sgi.com/mailman/listinfo/xfs" rel="noreferrer" target="_blank">http://oss.sgi.com/mailman/listinfo/xfs</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Jan Tulak<br></div><a href="mailto:jtulak@redhat.com" target="_blank">jtulak@redhat.com</a> / <a href="mailto:jan@tulak.me" target="_blank">jan@tulak.me</a></div></div></div></div>
</div></div>