xfs
[Top] [All Lists]

Re: [PATCH 04/17] mkfs: validate all input values

To: Jan ÅulÃk <jtulak@xxxxxxxxxx>
Subject: Re: [PATCH 04/17] mkfs: validate all input values
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Thu, 25 Jun 2015 15:38:14 -0400
Cc: xfs@xxxxxxxxxxx, Dave Chinner <dchinner@xxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1434711726-13092-5-git-send-email-jtulak@xxxxxxxxxx>
References: <1434711726-13092-1-git-send-email-jtulak@xxxxxxxxxx> <1434711726-13092-5-git-send-email-jtulak@xxxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
On Fri, Jun 19, 2015 at 01:01:53PM +0200, Jan ÅulÃk wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> Right now, mkfs does a poor job of input validation of values. Many
> parameters do not check for trailing garbage and so will pass
> obviously invalid values as OK. Some don't even detect completely
> invalid values, leaving it for other checks later on to fail due to
> a bad value conversion - these tend to rely on atoi() implicitly
> returning a sane value when it is passed garbage, and atoi gives no
> guarantee of the return value when passed garbage.
> 
> Clean all this up by passing all strings that need to be converted
> into values into a common function that is called regardless of
> whether unit conversion is needed or not. Further, make sure every
> conversion is checked for a valid result, and abort the moment an
> invalid value is detected.
> 
> Get rid of the silly "isdigits(), cvtnum()" calls which don't use
> any of the conversion capabilities of cvtnum() because we've already
> ensured that there are no conversion units in the string via the
> isdigits() call. These can simply be replaced by a standard
> strtoll() call followed by checking for no trailing bytes.
> 
> Finally, the block size of the filesystem is not known until all
> the options have been parsed and we can determine if the default is
> to be used. This means any parameter that relies on using conversion
> from filesystem block size (the "NNNb" format) requires the block
> size to first be specified on the command line so it is known.
> 
> Similarly, we make the same rule for specifying counts in sectors.
> This is a change from the existing behaviour that assumes sectors
> are 512 bytes unless otherwise changed on the command line. This,
> unfortunately, leads to complete silliness where you can specify the
> sector size as a count of sectors. It also means that you can do
> some conversions with 512 byte sector sizes, and others with
> whatever was specified on the command line, meaning the mkfs
> behaviour changes depending in where in the command line the sector
> size is changed....
> 
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> Signed-off-by: Jan ÅulÃk <jtulak@xxxxxxxxxx>
> ---

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

>  include/xfs_mkfs.h  |   7 +-
>  man/man8/mkfs.xfs.8 |  26 ++++++-
>  mkfs/proto.c        |  36 ++++-----
>  mkfs/xfs_mkfs.c     | 215 
> +++++++++++++++++++++++++++-------------------------
>  4 files changed, 153 insertions(+), 131 deletions(-)
> 
> diff --git a/include/xfs_mkfs.h b/include/xfs_mkfs.h
> index 3af9cb1..996b690 100644
> --- a/include/xfs_mkfs.h
> +++ b/include/xfs_mkfs.h
> @@ -56,11 +56,8 @@
>  #define XFS_NOMULTIDISK_AGLOG                2       /* 4 AGs */
>  #define XFS_MULTIDISK_AGCOUNT                (1 << XFS_MULTIDISK_AGLOG)
>  
> -
> -/* xfs_mkfs.c */
> -extern int isdigits (char *str);
> -extern long long cvtnum (unsigned int blocksize,
> -                      unsigned int sectorsize, char *s);
> +extern long long getnum(const char *str, unsigned int blocksize,
> +                     unsigned int sectorsize, bool convert);
>  
>  /* proto.c */
>  extern char *setup_proto (char *fname);
> diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8
> index 6260e0c..4456931 100644
> --- a/man/man8/mkfs.xfs.8
> +++ b/man/man8/mkfs.xfs.8
> @@ -64,11 +64,11 @@ SCSI disk, use:
>  .PP
>  The metadata log can be placed on another device to reduce the number
>  of disk seeks.  To create a filesystem on the first partition on the
> -first SCSI disk with a 10000 block log located on the first partition
> +first SCSI disk with a 10MiB log located on the first partition
>  on the second SCSI disk, use:
>  .RS
>  .HP
> -.B mkfs.xfs\ \-l\ logdev=/dev/sdb1,size=10000b /dev/sda1
> +.B mkfs.xfs\ \-l\ logdev=/dev/sdb1,size=10m /dev/sda1
>  .RE
>  .PP
>  Each of the
> @@ -78,9 +78,9 @@ suboptions if multiple suboptions apply to the same option.
>  Equivalently, each main option can be given multiple times with
>  different suboptions.
>  For example,
> -.B \-l internal,size=10000b
> +.B \-l internal,size=10m
>  and
> -.B \-l internal \-l size=10000b
> +.B \-l internal \-l size=10m
>  are equivalent.
>  .PP
>  In the descriptions below, sizes are given in sectors, bytes, blocks,
> @@ -109,6 +109,15 @@ option below).
>  .HP
>  .BR e "\ \-\ multiply by one exabyte (1,048,576 terabytes)."
>  .PD
> +.RE
> +.PP
> +When specifying parameters in units of sectors or filesystem blocks, the
> +.B \-s
> +option or the
> +.B \-b
> +option first needs to be added to the command line.
> +Failure to specify the size of the units will result in illegal value errors
> +when parameters are quantified in those units.
>  .SH OPTIONS
>  .TP
>  .BI \-b " block_size_options"
> @@ -126,6 +135,11 @@ or in bytes with
>  .BR size= .
>  The default value is 4096 bytes (4 KiB), the minimum is 512, and the
>  maximum is 65536 (64 KiB).
> +.IP
> +To specify any options on the command line in units of filesystem blocks, 
> this
> +option must be specified first so that the filesystem block size is
> +applied consistently to all options.
> +.IP
>  XFS on Linux currently only supports pagesize or smaller blocks.
>  .TP
>  .BI \-m " global_metadata_options"
> @@ -804,6 +818,10 @@ is 512 bytes. The minimum value for sector size is
>  .I sector_size
>  must be a power of 2 size and cannot be made larger than the
>  filesystem block size.
> +.IP
> +To specify any options on the command line in units of sectors, this
> +option must be specified first so that the sector size is
> +applied consistently to all options.
>  .TP
>  .BI \-L " label"
>  Set the filesystem
> diff --git a/mkfs/proto.c b/mkfs/proto.c
> index 45565b7..a2a61e0 100644
> --- a/mkfs/proto.c
> +++ b/mkfs/proto.c
> @@ -23,7 +23,6 @@
>  /*
>   * Prototypes for internal functions.
>   */
> -static long getnum(char **pp);
>  static char *getstr(char **pp);
>  static void fail(char *msg, int i);
>  static void getres(xfs_trans_t *tp, uint blocks);
> @@ -78,8 +77,8 @@ setup_proto(
>        * Skip past the stuff there for compatibility, a string and 2 numbers.
>        */
>       (void)getstr(&buf);     /* boot image name */
> -     (void)getnum(&buf);     /* block count */
> -     (void)getnum(&buf);     /* inode count */
> +     (void)getnum(getstr(&buf), 0, 0, false);        /* block count */
> +     (void)getnum(getstr(&buf), 0, 0, false);        /* inode count */
>       close(fd);
>       return buf;
>  
> @@ -90,16 +89,6 @@ out_fail:
>       exit(1);
>  }
>  
> -static long
> -getnum(
> -     char    **pp)
> -{
> -     char    *s;
> -
> -     s = getstr(pp);
> -     return atol(s);
> -}
> -
>  static void
>  fail(
>       char    *msg,
> @@ -441,8 +430,8 @@ parseproto(
>               val = val * 8 + mstr[i] - '0';
>       }
>       mode |= val;
> -     creds.cr_uid = (int)getnum(pp);
> -     creds.cr_gid = (int)getnum(pp);
> +     creds.cr_uid = getnum(getstr(pp), 0, 0, false);
> +     creds.cr_gid = getnum(getstr(pp), 0, 0, false);
>       xname.name = (uchar_t *)name;
>       xname.len = name ? strlen(name) : 0;
>       xname.type = 0;
> @@ -467,7 +456,14 @@ parseproto(
>  
>       case IF_RESERVED:                       /* pre-allocated space only */
>               value = getstr(pp);
> -             llen = cvtnum(mp->m_sb.sb_blocksize, mp->m_sb.sb_sectsize, 
> value);
> +             llen = getnum(value, mp->m_sb.sb_blocksize,
> +                           mp->m_sb.sb_sectsize, true);
> +             if (llen < 0) {
> +                     fprintf(stderr,
> +                             _("%s: Bad value %s for proto file %s\n"),
> +                             progname, value, name);
> +                     exit(1);
> +             }
>               getres(tp, XFS_B_TO_FSB(mp, llen));
>  
>               error = -libxfs_inode_alloc(&tp, pip, mode|S_IFREG, 1, 0,
> @@ -491,8 +487,8 @@ parseproto(
>  
>       case IF_BLOCK:
>               getres(tp, 0);
> -             majdev = (int)getnum(pp);
> -             mindev = (int)getnum(pp);
> +             majdev = getnum(getstr(pp), 0, 0, false);
> +             mindev = getnum(getstr(pp), 0, 0, false);
>               error = -libxfs_inode_alloc(&tp, pip, mode|S_IFBLK, 1,
>                               IRIX_MKDEV(majdev, mindev), &creds, fsxp, &ip);
>               if (error) {
> @@ -506,8 +502,8 @@ parseproto(
>  
>       case IF_CHAR:
>               getres(tp, 0);
> -             majdev = (int)getnum(pp);
> -             mindev = (int)getnum(pp);
> +             majdev = getnum(getstr(pp), 0, 0, false);
> +             mindev = getnum(getstr(pp), 0, 0, false);
>               error = -libxfs_inode_alloc(&tp, pip, mode|S_IFCHR, 1,
>                               IRIX_MKDEV(majdev, mindev), &creds, fsxp, &ip);
>               if (error)
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 10276e4..1a5e2f8 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -49,6 +49,9 @@ static void respec(char opt, char *tab[], int idx);
>  static void unknown(char opt, char *s);
>  static int  ispow2(unsigned int i);
>  
> +static long long cvtnum(unsigned int blocksize,
> +                     unsigned int sectorsize, const char *s);
> +
>  /*
>   * option tables for getsubopt calls
>   */
> @@ -1004,6 +1007,28 @@ sb_set_features(
>  
>  }
>  
> +long long
> +getnum(
> +     const char      *str,
> +     unsigned int    blocksize,
> +     unsigned int    sectorsize,
> +     bool            convert)
> +{
> +     long long       i;
> +     char            *sp;
> +
> +     if (convert)
> +             return cvtnum(blocksize, sectorsize, str);
> +
> +     i = strtoll(str, &sp, 0);
> +     if (i == 0 && sp == str)
> +             return -1LL;
> +     if (*sp != '\0')
> +             return -1LL; /* trailing garbage */
> +     return i;
> +}
> +
> +
>  int
>  main(
>       int                     argc,
> @@ -1123,8 +1148,8 @@ main(
>  
>       blflag = bsflag = slflag = ssflag = lslflag = lssflag = 0;
>       blocklog = blocksize = 0;
> -     sectorlog = lsectorlog = XFS_MIN_SECTORSIZE_LOG;
> -     sectorsize = lsectorsize = XFS_MIN_SECTORSIZE;
> +     sectorlog = lsectorlog = 0;
> +     sectorsize = lsectorsize = 0;
>       agsize = daflag = dasize = dblocks = 0;
>       ilflag = imflag = ipflag = isflag = 0;
>       liflag = laflag = lsflag = lsuflag = lsunitflag = ldflag = lvflag = 0;
> @@ -1167,7 +1192,7 @@ main(
>                                       if (bsflag)
>                                               conflict('b', bopts, B_SIZE,
>                                                        B_LOG);
> -                                     blocklog = atoi(value);
> +                                     blocklog = getnum(value, 0, 0, false);
>                                       if (blocklog <= 0)
>                                               illegal(value, "b log");
>                                       blocksize = 1 << blocklog;
> @@ -1181,8 +1206,8 @@ main(
>                                       if (blflag)
>                                               conflict('b', bopts, B_LOG,
>                                                        B_SIZE);
> -                                     blocksize = cvtnum(
> -                                             blocksize, sectorsize, value);
> +                                     blocksize = getnum(value, blocksize,
> +                                                     sectorsize, true);
>                                       if (blocksize <= 0 ||
>                                           !ispow2(blocksize))
>                                               illegal(value, "b size");
> @@ -1205,8 +1230,7 @@ main(
>                                               reqval('d', dopts, D_AGCOUNT);
>                                       if (daflag)
>                                               respec('d', dopts, D_AGCOUNT);
> -                                     agcount = (__uint64_t)
> -                                             strtoul(value, NULL, 10);
> +                                     agcount = getnum(value, 0, 0, false);
>                                       if ((__int64_t)agcount <= 0)
>                                               illegal(value, "d agcount");
>                                       daflag = 1;
> @@ -1216,14 +1240,16 @@ main(
>                                               reqval('d', dopts, D_AGSIZE);
>                                       if (dasize)
>                                               respec('d', dopts, D_AGSIZE);
> -                                     agsize = cvtnum(
> -                                             blocksize, sectorsize, value);
> +                                     agsize = getnum(value, blocksize,
> +                                                     sectorsize, true);
> +                                     if ((__int64_t)agsize <= 0)
> +                                             illegal(value, "d agsize");
>                                       dasize = 1;
>                                       break;
>                               case D_FILE:
>                                       if (!value || *value == '\0')
>                                               value = "1";
> -                                     xi.disfile = atoi(value);
> +                                     xi.disfile = getnum(value, 0, 0, false);
>                                       if (xi.disfile < 0 || xi.disfile > 1)
>                                               illegal(value, "d file");
>                                       if (xi.disfile && !Nflag)
> @@ -1251,13 +1277,9 @@ main(
>                                       if (nodsflag)
>                                               conflict('d', dopts, D_NOALIGN,
>                                                        D_SUNIT);
> -                                     if (!isdigits(value)) {
> -                                             fprintf(stderr,
> -     _("%s: Specify data sunit in 512-byte blocks, no unit suffix\n"),
> -                                                     progname);
> -                                             exit(1);
> -                                     }
> -                                     dsunit = cvtnum(0, 0, value);
> +                                     dsunit = getnum(value, 0, 0, false);
> +                                     if (dsunit < 0)
> +                                             illegal(value, "d sunit");
>                                       break;
>                               case D_SWIDTH:
>                                       if (!value || *value == '\0')
> @@ -1267,13 +1289,9 @@ main(
>                                       if (nodsflag)
>                                               conflict('d', dopts, D_NOALIGN,
>                                                        D_SWIDTH);
> -                                     if (!isdigits(value)) {
> -                                             fprintf(stderr,
> -     _("%s: Specify data swidth in 512-byte blocks, no unit suffix\n"),
> -                                                     progname);
> -                                             exit(1);
> -                                     }
> -                                     dswidth = cvtnum(0, 0, value);
> +                                     dswidth = getnum(value, 0, 0, false);
> +                                     if (dswidth < 0)
> +                                             illegal(value, "d swidth");
>                                       break;
>                               case D_SU:
>                                       if (!value || *value == '\0')
> @@ -1283,8 +1301,10 @@ main(
>                                       if (nodsflag)
>                                               conflict('d', dopts, D_NOALIGN,
>                                                        D_SU);
> -                                     dsu = cvtnum(
> -                                             blocksize, sectorsize, value);
> +                                     dsu = getnum(value, blocksize,
> +                                                  sectorsize, true);
> +                                     if (dsu < 0)
> +                                             illegal(value, "d su");
>                                       break;
>                               case D_SW:
>                                       if (!value || *value == '\0')
> @@ -1294,13 +1314,9 @@ main(
>                                       if (nodsflag)
>                                               conflict('d', dopts, D_NOALIGN,
>                                                        D_SW);
> -                                     if (!isdigits(value)) {
> -                                             fprintf(stderr,
> -             _("%s: Specify data sw as multiple of su, no unit suffix\n"),
> -                                                     progname);
> -                                             exit(1);
> -                                     }
> -                                     dsw = cvtnum(0, 0, value);
> +                                     dsw = getnum(value, 0, 0, false);
> +                                     if (dsw < 0)
> +                                             illegal(value, "d sw");
>                                       break;
>                               case D_NOALIGN:
>                                       if (dsu)
> @@ -1325,7 +1341,7 @@ main(
>                                       if (ssflag)
>                                               conflict('d', dopts, D_SECTSIZE,
>                                                        D_SECTLOG);
> -                                     sectorlog = atoi(value);
> +                                     sectorlog = getnum(value, 0, 0, false);
>                                       if (sectorlog <= 0)
>                                               illegal(value, "d sectlog");
>                                       sectorsize = 1 << sectorlog;
> @@ -1339,8 +1355,8 @@ main(
>                                       if (slflag)
>                                               conflict('d', dopts, D_SECTLOG,
>                                                        D_SECTSIZE);
> -                                     sectorsize = cvtnum(
> -                                             blocksize, sectorsize, value);
> +                                     sectorsize = getnum(value, blocksize,
> +                                                         sectorsize, true);
>                                       if (sectorsize <= 0 ||
>                                           !ispow2(sectorsize))
>                                               illegal(value, "d sectsize");
> @@ -1380,7 +1396,7 @@ main(
>                               case I_ALIGN:
>                                       if (!value || *value == '\0')
>                                               break;
> -                                     c = atoi(value);
> +                                     c = getnum(value, 0, 0, false);
>                                       if (c < 0 || c > 1)
>                                               illegal(value, "i align");
>                                       sb_feat.inode_align = c ? true : false;
> @@ -1396,7 +1412,7 @@ main(
>                                       if (isflag)
>                                               conflict('i', iopts, I_SIZE,
>                                                        I_LOG);
> -                                     inodelog = atoi(value);
> +                                     inodelog = getnum(value, 0, 0, false);
>                                       if (inodelog <= 0)
>                                               illegal(value, "i log");
>                                       isize = 1 << inodelog;
> @@ -1407,7 +1423,7 @@ main(
>                                               reqval('i', iopts, I_MAXPCT);
>                                       if (imflag)
>                                               respec('i', iopts, I_MAXPCT);
> -                                     imaxpct = atoi(value);
> +                                     imaxpct = getnum(value, 0, 0, false);
>                                       if (imaxpct < 0 || imaxpct > 100)
>                                               illegal(value, "i maxpct");
>                                       imflag = 1;
> @@ -1423,7 +1439,7 @@ main(
>                                       if (isflag)
>                                               conflict('i', iopts, I_SIZE,
>                                                        I_PERBLOCK);
> -                                     inopblock = atoi(value);
> +                                     inopblock = getnum(value, 0, 0, false);
>                                       if (inopblock <
>                                               XFS_MIN_INODE_PERBLOCK ||
>                                           !ispow2(inopblock))
> @@ -1441,7 +1457,7 @@ main(
>                                                        I_SIZE);
>                                       if (isflag)
>                                               respec('i', iopts, I_SIZE);
> -                                     isize = cvtnum(0, 0, value);
> +                                     isize = getnum(value, 0, 0, true);
>                                       if (isize <= 0 || !ispow2(isize))
>                                               illegal(value, "i size");
>                                       inodelog = libxfs_highbit32(isize);
> @@ -1450,7 +1466,7 @@ main(
>                               case I_ATTR:
>                                       if (!value || *value == '\0')
>                                               reqval('i', iopts, I_ATTR);
> -                                     c = atoi(value);
> +                                     c = getnum(value, 0, 0, false);
>                                       if (c < 0 || c > 2)
>                                               illegal(value, "i attr");
>                                       sb_feat.attr_version = c;
> @@ -1458,7 +1474,7 @@ main(
>                               case I_PROJID32BIT:
>                                       if (!value || *value == '\0')
>                                               value = "0";
> -                                     c = atoi(value);
> +                                     c = getnum(value, 0, 0, false);
>                                       if (c < 0 || c > 1)
>                                               illegal(value, "i projid32bit");
>                                       sb_feat.projid16bit = c ? false : true;
> @@ -1488,7 +1504,9 @@ main(
>                                               respec('l', lopts, L_AGNUM);
>                                       if (ldflag)
>                                               conflict('l', lopts, L_AGNUM, 
> L_DEV);
> -                                     logagno = atoi(value);
> +                                     logagno = getnum(value, 0, 0, false);
> +                                     if (logagno < 0)
> +                                             illegal(value, "l agno");
>                                       laflag = 1;
>                                       break;
>                               case L_FILE:
> @@ -1497,7 +1515,7 @@ main(
>                                       if (loginternal)
>                                               conflict('l', lopts, L_INTERNAL,
>                                                        L_FILE);
> -                                     xi.lisfile = atoi(value);
> +                                     xi.lisfile = getnum(value, 0, 0, false);
>                                       if (xi.lisfile < 0 || xi.lisfile > 1)
>                                               illegal(value, "l file");
>                                       if (xi.lisfile)
> @@ -1513,7 +1531,7 @@ main(
>                                                        L_INTERNAL);
>                                       if (liflag)
>                                               respec('l', lopts, L_INTERNAL);
> -                                     loginternal = atoi(value);
> +                                     loginternal = getnum(value, 0, 0, 
> false);
>                                       if (loginternal < 0 || loginternal > 1)
>                                               illegal(value, "l internal");
>                                       liflag = 1;
> @@ -1523,8 +1541,10 @@ main(
>                                               reqval('l', lopts, L_SU);
>                                       if (lsu)
>                                               respec('l', lopts, L_SU);
> -                                     lsu = cvtnum(
> -                                             blocksize, sectorsize, value);
> +                                     lsu = getnum(value, blocksize,
> +                                                  sectorsize, true);
> +                                     if (lsu < 0)
> +                                             illegal(value, "l su");
>                                       lsuflag = 1;
>                                       break;
>                               case L_SUNIT:
> @@ -1532,12 +1552,9 @@ main(
>                                               reqval('l', lopts, L_SUNIT);
>                                       if (lsunit)
>                                               respec('l', lopts, L_SUNIT);
> -                                     if (!isdigits(value)) {
> -                                             fprintf(stderr,
> -             _("Specify log sunit in 512-byte blocks, no size suffix\n"));
> -                                             usage();
> -                                     }
> -                                     lsunit = cvtnum(0, 0, value);
> +                                     lsunit = getnum(value, 0, 0, false);
> +                                     if (lsunit < 0)
> +                                             illegal(value, "l sunit");
>                                       lsunitflag = 1;
>                                       break;
>                               case L_NAME:
> @@ -1560,7 +1577,7 @@ main(
>                                               reqval('l', lopts, L_VERSION);
>                                       if (lvflag)
>                                               respec('l', lopts, L_VERSION);
> -                                     c = atoi(value);
> +                                     c = getnum(value, 0, 0, false);
>                                       if (c < 1 || c > 2)
>                                               illegal(value, "l version");
>                                       sb_feat.log_version = c;
> @@ -1582,7 +1599,7 @@ main(
>                                       if (lssflag)
>                                               conflict('l', lopts, L_SECTSIZE,
>                                                        L_SECTLOG);
> -                                     lsectorlog = atoi(value);
> +                                     lsectorlog = getnum(value, 0, 0, false);
>                                       if (lsectorlog <= 0)
>                                               illegal(value, "l sectlog");
>                                       lsectorsize = 1 << lsectorlog;
> @@ -1596,8 +1613,8 @@ main(
>                                       if (lslflag)
>                                               conflict('l', lopts, L_SECTLOG,
>                                                        L_SECTSIZE);
> -                                     lsectorsize = cvtnum(
> -                                             blocksize, sectorsize, value);
> +                                     lsectorsize = getnum(value, blocksize,
> +                                                          sectorsize, true);
>                                       if (lsectorsize <= 0 ||
>                                           !ispow2(lsectorsize))
>                                               illegal(value, "l sectsize");
> @@ -1609,7 +1626,7 @@ main(
>                                       if (!value || *value == '\0')
>                                               reqval('l', lopts,
>                                                               L_LAZYSBCNTR);
> -                                     c = atoi(value);
> +                                     c = getnum(value, 0, 0, false);
>                                       if (c < 0 || c > 1)
>                                               illegal(value, "l lazy-count");
>                                       sb_feat.lazy_sb_counters = c ? true
> @@ -1634,7 +1651,7 @@ main(
>                               case M_CRC:
>                                       if (!value || *value == '\0')
>                                               reqval('m', mopts, M_CRC);
> -                                     c = atoi(value);
> +                                     c = getnum(value, 0, 0, false);
>                                       if (c < 0 || c > 1)
>                                               illegal(value, "m crc");
>                                       if (c && nftype) {
> @@ -1673,7 +1690,7 @@ _("cannot specify both crc and ftype\n"));
>                                       if (nsflag)
>                                               conflict('n', nopts, N_SIZE,
>                                                        N_LOG);
> -                                     dirblocklog = atoi(value);
> +                                     dirblocklog = getnum(value, 0, 0, 
> false);
>                                       if (dirblocklog <= 0)
>                                               illegal(value, "n log");
>                                       dirblocksize = 1 << dirblocklog;
> @@ -1687,8 +1704,8 @@ _("cannot specify both crc and ftype\n"));
>                                       if (nlflag)
>                                               conflict('n', nopts, N_LOG,
>                                                        N_SIZE);
> -                                     dirblocksize = cvtnum(
> -                                             blocksize, sectorsize, value);
> +                                     dirblocksize = getnum(value, blocksize,
> +                                                           sectorsize, true);
>                                       if (dirblocksize <= 0 ||
>                                           !ispow2(dirblocksize))
>                                               illegal(value, "n size");
> @@ -1705,7 +1722,7 @@ _("cannot specify both crc and ftype\n"));
>                                               /* ASCII CI mode */
>                                               sb_feat.nci = true;
>                                       } else {
> -                                             c = atoi(value);
> +                                             c = getnum(value, 0, 0, false);
>                                               if (c != 2)
>                                                       illegal(value,
>                                                               "n version");
> @@ -1718,7 +1735,7 @@ _("cannot specify both crc and ftype\n"));
>                                               reqval('n', nopts, N_FTYPE);
>                                       if (nftype)
>                                               respec('n', nopts, N_FTYPE);
> -                                     c = atoi(value);
> +                                     c = getnum(value, 0, 0, false);
>                                       if (c < 0 || c > 1)
>                                               illegal(value, "n ftype");
>                                       if (sb_feat.crcs_enabled) {
> @@ -1764,7 +1781,7 @@ _("cannot specify both crc and ftype\n"));
>                               case R_FILE:
>                                       if (!value || *value == '\0')
>                                               value = "1";
> -                                     xi.risfile = atoi(value);
> +                                     xi.risfile = getnum(value, 0, 0, false);
>                                       if (xi.risfile < 0 || xi.risfile > 1)
>                                               illegal(value, "r file");
>                                       if (xi.risfile)
> @@ -1808,7 +1825,7 @@ _("cannot specify both crc and ftype\n"));
>                                       if (ssflag || lssflag)
>                                               conflict('s', sopts, S_SECTSIZE,
>                                                        S_SECTLOG);
> -                                     sectorlog = atoi(value);
> +                                     sectorlog = getnum(value, 0, 0, false);
>                                       if (sectorlog <= 0)
>                                               illegal(value, "s sectlog");
>                                       lsectorlog = sectorlog;
> @@ -1825,8 +1842,8 @@ _("cannot specify both crc and ftype\n"));
>                                       if (slflag || lslflag)
>                                               conflict('s', sopts, S_SECTLOG,
>                                                        S_SECTSIZE);
> -                                     sectorsize = cvtnum(
> -                                             blocksize, sectorsize, value);
> +                                     sectorsize = getnum(value, blocksize,
> +                                                         sectorsize, true);
>                                       if (sectorsize <= 0 ||
>                                           !ispow2(sectorsize))
>                                               illegal(value, "s sectsize");
> @@ -1882,6 +1899,15 @@ _("Minimum block size for CRC enabled filesystems is 
> %d bytes.\n"),
>               usage();
>       }
>  
> +     if (!slflag && !ssflag) {
> +             sectorlog = XFS_MIN_SECTORSIZE_LOG;
> +             sectorsize = XFS_MIN_SECTORSIZE;
> +     }
> +     if (!lslflag && !lssflag) {
> +             lsectorlog = sectorlog;
> +             lsectorsize = sectorsize;
> +     }
> +
>       memset(&ft, 0, sizeof(ft));
>       get_topology(&xi, &ft, force_overwrite);
>  
> @@ -2055,7 +2081,9 @@ _("warning: sparse inodes not supported without CRC 
> support, disabled.\n"));
>       if (dsize) {
>               __uint64_t dbytes;
>  
> -             dbytes = cvtnum(blocksize, sectorsize, dsize);
> +             dbytes = getnum(dsize, blocksize, sectorsize, true);
> +             if ((__int64_t)dbytes < 0)
> +                     illegal(dsize, "d size");
>               if (dbytes % XFS_MIN_BLOCKSIZE) {
>                       fprintf(stderr,
>                       _("illegal data length %lld, not a multiple of %d\n"),
> @@ -2092,7 +2120,9 @@ _("warning: sparse inodes not supported without CRC 
> support, disabled.\n"));
>       if (logsize) {
>               __uint64_t logbytes;
>  
> -             logbytes = cvtnum(blocksize, sectorsize, logsize);
> +             logbytes = getnum(logsize, blocksize, sectorsize, true);
> +             if ((__int64_t)logbytes < 0)
> +                     illegal(logsize, "l size");
>               if (logbytes % XFS_MIN_BLOCKSIZE) {
>                       fprintf(stderr,
>                       _("illegal log length %lld, not a multiple of %d\n"),
> @@ -2114,7 +2144,9 @@ _("warning: sparse inodes not supported without CRC 
> support, disabled.\n"));
>       if (rtsize) {
>               __uint64_t rtbytes;
>  
> -             rtbytes = cvtnum(blocksize, sectorsize, rtsize);
> +             rtbytes = getnum(rtsize, blocksize, sectorsize, true);
> +             if ((__int64_t)rtbytes < 0)
> +                     illegal(rtsize, "r size");
>               if (rtbytes % XFS_MIN_BLOCKSIZE) {
>                       fprintf(stderr,
>                       _("illegal rt length %lld, not a multiple of %d\n"),
> @@ -2134,7 +2166,9 @@ _("warning: sparse inodes not supported without CRC 
> support, disabled.\n"));
>       if (rtextsize) {
>               __uint64_t rtextbytes;
>  
> -             rtextbytes = cvtnum(blocksize, sectorsize, rtextsize);
> +             rtextbytes = getnum(rtextsize, blocksize, sectorsize, true);
> +             if ((__int64_t)rtextbytes < 0)
> +                     illegal(rtsize, "r extsize");
>               if (rtextbytes % blocksize) {
>                       fprintf(stderr,
>               _("illegal rt extent size %lld, not a multiple of %d\n"),
> @@ -3240,28 +3274,11 @@ unknown(
>       usage();
>  }
>  
> -/*
> - * isdigits -- returns 1 if string contains nothing but [0-9], 0 otherwise
> - */
> -int
> -isdigits(
> -     char            *str)
> -{
> -     int             i;
> -     int             n = strlen(str);
> -
> -     for (i = 0; i < n; i++) {
> -             if (!isdigit((int)str[i]))
> -                     return 0;
> -     }
> -     return 1;
> -}
> -
>  long long
>  cvtnum(
>       unsigned int    blocksize,
>       unsigned int    sectorsize,
> -     char            *s)
> +     const char      *s)
>  {
>       long long       i;
>       char            *sp;
> @@ -3272,17 +3289,11 @@ cvtnum(
>       if (*sp == '\0')
>               return i;
>  
> -     if (*sp == 'b' && sp[1] == '\0') {
> -             if (blocksize)
> -                     return i * blocksize;
> -             fprintf(stderr, _("blocksize not available yet.\n"));
> -             usage();
> -     }
> -     if (*sp == 's' && sp[1] == '\0') {
> -             if (sectorsize)
> -                     return i * sectorsize;
> -             return i * BBSIZE;
> -     }
> +     if (*sp == 'b' && sp[1] == '\0')
> +             return i * blocksize;
> +     if (*sp == 's' && sp[1] == '\0')
> +             return i * sectorsize;
> +
>       if (*sp == 'k' && sp[1] == '\0')
>               return 1024LL * i;
>       if (*sp == 'm' && sp[1] == '\0')
> -- 
> 2.1.0
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>