On Fri, Jun 19, 2015 at 01:02:01PM +0200, Jan Ťulák wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> getnum() is now only called by getnum_checked(). Move the two
> together into a single getnum() function and change all the callers
> back to getnum().
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> Signed-off-by: Jan Ťulák <jtulak@xxxxxxxxxx>
> ---
> include/xfs_mkfs.h | 4 +-
> mkfs/proto.c | 20 ++++++
> mkfs/xfs_mkfs.c | 202 ++++++++++++++++++++++++-----------------------------
> 3 files changed, 113 insertions(+), 113 deletions(-)
>
> diff --git a/include/xfs_mkfs.h b/include/xfs_mkfs.h
> index f5639b0..a4312e1 100644
> --- a/include/xfs_mkfs.h
> +++ b/include/xfs_mkfs.h
> @@ -57,8 +57,8 @@
> #define XFS_NOMULTIDISK_AGLOG 2 /* 4 AGs */
> #define XFS_MULTIDISK_AGCOUNT (1 << XFS_MULTIDISK_AGLOG)
>
> -extern long long getnum(const char *str, unsigned int blocksize,
> - unsigned int sectorsize, bool convert);
> +extern long long cvtnum(unsigned int blksize, unsigned int sectsize,
> + const char *str);
>
> /* proto.c */
> extern char *setup_proto (char *fname);
> diff --git a/mkfs/proto.c b/mkfs/proto.c
> index a2a61e0..1e763a0 100644
> --- a/mkfs/proto.c
> +++ b/mkfs/proto.c
> @@ -43,6 +43,26 @@ static long filesize(int fd);
> ((uint)(MKFS_BLOCKRES_INODE + XFS_DA_NODE_MAXDEPTH + \
> (XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) - 1) + (rb)))
>
> +static long long
> +getnum(
> + const char *str,
> + unsigned int blksize,
> + unsigned int sectsize,
> + bool convert)
> +{
> + long long i;
> + char *sp;
> +
> + if (convert)
> + return cvtnum(blksize, sectsize, str);
> +
> + i = strtoll(str, &sp, 0);
> + if (i == 0 && sp == str)
> + return -1LL;
> + if (*sp != '\0')
> + return -1LL; /* trailing garbage */
> + return i;
> +}
>
> char *
> setup_proto(
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 4fc1f34..e52fd4e 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -49,9 +49,6 @@ 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);
> -
> /*
> * The configured block and sector sizes are defined as global variables so
> * that they don't need to be passed to functions that require them.
> @@ -1330,27 +1327,6 @@ sb_set_features(
>
> }
>
> -long long
> -getnum(
> - const char *str,
> - unsigned int blksize,
> - unsigned int sectsize,
> - bool convert)
> -{
> - long long i;
> - char *sp;
> -
> - if (convert)
> - return cvtnum(blksize, sectsize, str);
> -
> - i = strtoll(str, &sp, 0);
> - if (i == 0 && sp == str)
> - return -1LL;
> - if (*sp != '\0')
> - return -1LL; /* trailing garbage */
> - return i;
> -}
> -
> static __attribute__((noreturn)) void
> illegal_option(
> const char *value,
> @@ -1363,8 +1339,8 @@ illegal_option(
> usage();
> }
>
> -static int
> -getnum_checked(
> +static long long
> +getnum(
> const char *str,
> struct opt_params *opts,
> int index)
> @@ -1384,13 +1360,32 @@ getnum_checked(
> respec(opts->name, (char **)opts->subopts, index);
> sp->seen = true;
>
> + /* empty strings might just return a default value */
> if (!str || *str == '\0') {
> if (sp->defaultval == SUBOPT_NEEDS_VAL)
> reqval(opts->name, (char **)opts->subopts, index);
> return sp->defaultval;
> }
>
> - c = getnum(str, blocksize, sectorsize, sp->convert);
> + /*
> + * Some values are pure numbers, others can have suffixes that define
> + * the units of the number. Those get passed to cvtnum(), otherwise we
> + * convert it ourselves to guarantee there is no trailing garbage in the
> + * number.
> + */
> + if (sp->convert)
> + c = cvtnum(blocksize, sectorsize, str);
> + else {
> + char *sp;
> +
> + c = strtoll(str, &sp, 0);
> + if (c == 0 && sp == str)
> + illegal_option(str, opts, index);
> + if (*sp != '\0')
> + illegal_option(str, opts, index);
> + }
> +
Hrm, why not continue to reuse this code? E.g., give the getnum() from
proto.c a better name and call that. Otherwise, this seems fine.
Brian
> + /* Validity check the result. */
> if (c < sp->minval || c > sp->maxval)
> illegal_option(str, opts, index);
> if (sp->is_power_2 && !ispow2(c))
> @@ -1556,8 +1551,7 @@ main(
> if (bsflag)
> conflict('b', subopts, B_SIZE,
> B_LOG);
> - blocklog = getnum_checked(value, &bopts,
> - B_LOG);
> + blocklog = getnum(value, &bopts, B_LOG);
> blocksize = 1 << blocklog;
> blflag = 1;
> break;
> @@ -1565,8 +1559,8 @@ main(
> if (blflag)
> conflict('b', subopts, B_LOG,
> B_SIZE);
> - blocksize = getnum_checked(value, &bopts,
> - B_SIZE);
> + blocksize = getnum(value, &bopts,
> + B_SIZE);
> blocklog = libxfs_highbit32(blocksize);
> bsflag = 1;
> break;
> @@ -1584,18 +1578,17 @@ main(
> switch (getsubopt(&p, (constpp)subopts,
> &value)) {
> case D_AGCOUNT:
> - agcount = getnum_checked(value, &dopts,
> - D_AGCOUNT);
> + agcount = getnum(value, &dopts,
> + D_AGCOUNT);
> daflag = 1;
> break;
> case D_AGSIZE:
> - agsize = getnum_checked(value, &dopts,
> - D_AGSIZE);
> + agsize = getnum(value, &dopts, D_AGSIZE);
> dasize = 1;
> break;
> case D_FILE:
> - xi.disfile = getnum_checked(value,
> - &dopts, D_FILE);
> + xi.disfile = getnum(value, &dopts,
> + D_FILE);
> if (xi.disfile && !Nflag)
> xi.dcreat = 1;
> break;
> @@ -1617,29 +1610,26 @@ main(
> if (nodsflag)
> conflict('d', subopts, D_NOALIGN,
> D_SUNIT);
> - dsunit = getnum_checked(value, &dopts,
> - D_SUNIT);
> + dsunit = getnum(value, &dopts, D_SUNIT);
> break;
> case D_SWIDTH:
> if (nodsflag)
> conflict('d', subopts, D_NOALIGN,
> D_SWIDTH);
> - dswidth = getnum_checked(value, &dopts,
> - D_SWIDTH);
> + dswidth = getnum(value, &dopts,
> + D_SWIDTH);
> break;
> case D_SU:
> if (nodsflag)
> conflict('d', subopts, D_NOALIGN,
> D_SU);
> - dsu = getnum_checked(value, &dopts,
> - D_SU);
> + dsu = getnum(value, &dopts, D_SU);
> break;
> case D_SW:
> if (nodsflag)
> conflict('d', subopts, D_NOALIGN,
> D_SW);
> - dsw = getnum_checked(value, &dopts,
> - D_SW);
> + dsw = getnum(value, &dopts, D_SW);
> break;
> case D_NOALIGN:
> if (dsu)
> @@ -1660,8 +1650,8 @@ main(
> if (ssflag)
> conflict('d', subopts, D_SECTSIZE,
> D_SECTLOG);
> - sectorlog = getnum_checked(value, &dopts,
> - D_SECTLOG);
> + sectorlog = getnum(value, &dopts,
> + D_SECTLOG);
> sectorsize = 1 << sectorlog;
> slflag = 1;
> break;
> @@ -1669,28 +1659,27 @@ main(
> if (slflag)
> conflict('d', subopts, D_SECTLOG,
> D_SECTSIZE);
> - sectorsize = getnum_checked(value,
> - &dopts, D_SECTSIZE);
> + sectorsize = getnum(value, &dopts,
> + D_SECTSIZE);
> sectorlog =
> libxfs_highbit32(sectorsize);
> ssflag = 1;
> break;
> case D_RTINHERIT:
> - c = getnum_checked(value, &dopts,
> - D_RTINHERIT);
> + c = getnum(value, &dopts, D_RTINHERIT);
> if (c)
> fsx.fsx_xflags |=
> XFS_DIFLAG_RTINHERIT;
> break;
> case D_PROJINHERIT:
> - fsx.fsx_projid = getnum_checked(value,
> - &dopts, D_PROJINHERIT);
> + fsx.fsx_projid = getnum(value, &dopts,
> + D_PROJINHERIT);
> fsx.fsx_xflags |=
> XFS_DIFLAG_PROJINHERIT;
> break;
> case D_EXTSZINHERIT:
> - fsx.fsx_extsize = getnum_checked(value,
> - &dopts, D_EXTSZINHERIT);
> + fsx.fsx_extsize = getnum(value, &dopts,
> + D_EXTSZINHERIT);
> fsx.fsx_xflags |=
> XFS_DIFLAG_EXTSZINHERIT;
> break;
> @@ -1708,8 +1697,8 @@ main(
> switch (getsubopt(&p, (constpp)subopts,
> &value)) {
> case I_ALIGN:
> - sb_feat.inode_align = getnum_checked(
> - value, &iopts, I_ALIGN);
> + sb_feat.inode_align = getnum(value,
> + &iopts, I_ALIGN);
> break;
> case I_LOG:
> if (ipflag)
> @@ -1718,14 +1707,13 @@ main(
> if (isflag)
> conflict('i', subopts, I_SIZE,
> I_LOG);
> - inodelog = getnum_checked(value, &iopts,
> - I_LOG);
> + inodelog = getnum(value, &iopts, I_LOG);
> isize = 1 << inodelog;
> ilflag = 1;
> break;
> case I_MAXPCT:
> - imaxpct = getnum_checked(value, &iopts,
> - I_MAXPCT);
> + imaxpct = getnum(value, &iopts,
> + I_MAXPCT);
> imflag = 1;
> break;
> case I_PERBLOCK:
> @@ -1735,8 +1723,8 @@ main(
> if (isflag)
> conflict('i', subopts, I_SIZE,
> I_PERBLOCK);
> - inopblock = getnum_checked(value, &iopts,
> - I_PERBLOCK);
> + inopblock = getnum(value, &iopts,
> + I_PERBLOCK);
> ipflag = 1;
> break;
> case I_SIZE:
> @@ -1746,20 +1734,18 @@ main(
> if (ipflag)
> conflict('i', subopts, I_PERBLOCK,
> I_SIZE);
> - isize = getnum_checked(value, &iopts,
> - I_SIZE);
> + isize = getnum(value, &iopts, I_SIZE);
> inodelog = libxfs_highbit32(isize);
> isflag = 1;
> break;
> case I_ATTR:
> sb_feat.attr_version =
> - getnum_checked(value, &iopts,
> - I_ATTR);
> + getnum(value, &iopts, I_ATTR);
> break;
> case I_PROJID32BIT:
> sb_feat.projid16bit =
> - !getnum_checked(value, &iopts,
> - I_PROJID32BIT);
> + !getnum(value, &iopts,
> + I_PROJID32BIT);
> break;
> case I_SPINODES:
> if (!value || *value == '\0')
> @@ -1784,16 +1770,15 @@ main(
> case L_AGNUM:
> if (ldflag)
> conflict('l', subopts, L_AGNUM, L_DEV);
> - logagno = getnum_checked(value, &lopts,
> - L_AGNUM);
> + logagno = getnum(value, &lopts, L_AGNUM);
> laflag = 1;
> break;
> case L_FILE:
> if (loginternal)
> conflict('l', subopts, L_INTERNAL,
> L_FILE);
> - xi.lisfile = getnum_checked(value,
> - &lopts, L_FILE);
> + xi.lisfile = getnum(value, &lopts,
> + L_FILE);
> if (xi.lisfile)
> xi.lcreat = 1;
> break;
> @@ -1804,18 +1789,16 @@ main(
> conflict('l', subopts, L_FILE,
> L_INTERNAL);
>
> - loginternal = getnum_checked(value,
> - &lopts, L_INTERNAL);
> + loginternal = getnum(value, &lopts,
> + L_INTERNAL);
> liflag = 1;
> break;
> case L_SU:
> - lsu = getnum_checked(value, &lopts,
> - L_SU);
> + lsu = getnum(value, &lopts, L_SU);
> lsuflag = 1;
> break;
> case L_SUNIT:
> - lsunit = getnum_checked(value, &lopts,
> - L_SUNIT);
> + lsunit = getnum(value, &lopts, L_SUNIT);
> lsunitflag = 1;
> break;
> case L_NAME:
> @@ -1835,8 +1818,7 @@ main(
> break;
> case L_VERSION:
> sb_feat.log_version =
> - getnum_checked(value, &lopts,
> - L_VERSION);
> + getnum(value, &lopts, L_VERSION);
> lvflag = 1;
> break;
> case L_SIZE:
> @@ -1851,8 +1833,8 @@ main(
> if (lssflag)
> conflict('l', subopts, L_SECTSIZE,
> L_SECTLOG);
> - lsectorlog = getnum_checked(value,
> - &lopts, L_SECTLOG);
> + lsectorlog = getnum(value, &lopts,
> + L_SECTLOG);
> lsectorsize = 1 << lsectorlog;
> lslflag = 1;
> break;
> @@ -1860,15 +1842,15 @@ main(
> if (lslflag)
> conflict('l', subopts, L_SECTLOG,
> L_SECTSIZE);
> - lsectorsize = getnum_checked(value,
> - &lopts, L_SECTSIZE);
> + lsectorsize = getnum(value, &lopts,
> + L_SECTSIZE);
> lsectorlog =
> libxfs_highbit32(lsectorsize);
> lssflag = 1;
> break;
> case L_LAZYSBCNTR:
> sb_feat.lazy_sb_counters =
> - getnum_checked(value, &lopts,
> + getnum(value, &lopts,
> L_LAZYSBCNTR);
> break;
> default:
> @@ -1891,8 +1873,7 @@ main(
> &value)) {
> case M_CRC:
> sb_feat.crcs_enabled =
> - getnum_checked(value, &mopts,
> - M_CRC);
> + getnum(value, &mopts, M_CRC);
> if (sb_feat.crcs_enabled && nftype) {
> fprintf(stderr,
> _("cannot specify both -m crc=1 and -n ftype\n"));
> @@ -1926,8 +1907,8 @@ _("cannot specify both -m crc=1 and -n ftype\n"));
> if (nsflag)
> conflict('n', subopts, N_SIZE,
> N_LOG);
> - dirblocklog = getnum_checked(value,
> - &nopts, N_LOG);
> + dirblocklog = getnum(value, &nopts,
> + N_LOG);
> dirblocksize = 1 << dirblocklog;
> nlflag = 1;
> break;
> @@ -1935,8 +1916,8 @@ _("cannot specify both -m crc=1 and -n ftype\n"));
> if (nlflag)
> conflict('n', subopts, N_LOG,
> N_SIZE);
> - dirblocksize = getnum_checked(value,
> - &nopts, N_SIZE);
> + dirblocksize = getnum(value, &nopts,
> + N_SIZE);
> dirblocklog =
> libxfs_highbit32(dirblocksize);
> nsflag = 1;
> @@ -1951,9 +1932,8 @@ _("cannot specify both -m crc=1 and -n ftype\n"));
> sb_feat.nci = true;
> } else {
> sb_feat.dir_version =
> - getnum_checked(value,
> - &nopts,
> - N_VERSION);
> + getnum(value, &nopts,
> + N_VERSION);
> }
> nvflag = 1;
> break;
> @@ -1963,8 +1943,8 @@ _("cannot specify both -m crc=1 and -n ftype\n"));
> _("cannot specify both -m crc=1 and -n ftype\n"));
> usage();
> }
> - sb_feat.dirftype = getnum_checked(value,
> - &nopts, N_FTYPE);
> + sb_feat.dirftype = getnum(value, &nopts,
> + N_FTYPE);
> nftype = 1;
> break;
> default:
> @@ -2002,8 +1982,8 @@ _("cannot specify both -m crc=1 and -n ftype\n"));
> rtextsize = value;
> break;
> case R_FILE:
> - xi.risfile = getnum_checked(value,
> - &ropts, R_FILE);
> + xi.risfile = getnum(value, &ropts,
> + R_FILE);
> if (xi.risfile)
> xi.rcreat = 1;
> break;
> @@ -2043,8 +2023,8 @@ _("cannot specify both -m crc=1 and -n ftype\n"));
> if (ssflag || lssflag)
> conflict('s', subopts,
> S_SECTSIZE, S_SECTLOG);
> - sectorlog = getnum_checked(value, &sopts,
> - S_SECTLOG);
> + sectorlog = getnum(value, &sopts,
> + S_SECTLOG);
> lsectorlog = sectorlog;
> sectorsize = 1 << sectorlog;
> lsectorsize = sectorsize;
> @@ -2055,8 +2035,8 @@ _("cannot specify both -m crc=1 and -n ftype\n"));
> if (slflag || lslflag)
> conflict('s', subopts, S_SECTLOG,
> S_SECTSIZE);
> - sectorsize = getnum_checked(value,
> - &sopts, S_SECTSIZE);
> + sectorsize = getnum(value, &sopts,
> + S_SECTSIZE);
> lsectorsize = sectorsize;
> sectorlog =
> libxfs_highbit32(sectorsize);
> @@ -2291,7 +2271,7 @@ _("warning: sparse inodes not supported without CRC support, disabled.\n"));
> if (dsize) {
> __uint64_t dbytes;
>
> - dbytes = getnum_checked(dsize, &dopts, D_SIZE);
> + dbytes = getnum(dsize, &dopts, D_SIZE);
> if (dbytes % XFS_MIN_BLOCKSIZE) {
> fprintf(stderr,
> _("illegal data length %lld, not a multiple of %d\n"),
> @@ -2328,7 +2308,7 @@ _("warning: sparse inodes not supported without CRC support, disabled.\n"));
> if (logsize) {
> __uint64_t logbytes;
>
> - logbytes = getnum_checked(logsize, &lopts, L_SIZE);
> + logbytes = getnum(logsize, &lopts, L_SIZE);
> if (logbytes % XFS_MIN_BLOCKSIZE) {
> fprintf(stderr,
> _("illegal log length %lld, not a multiple of %d\n"),
> @@ -2350,7 +2330,7 @@ _("warning: sparse inodes not supported without CRC support, disabled.\n"));
> if (rtsize) {
> __uint64_t rtbytes;
>
> - rtbytes = getnum_checked(rtsize, &ropts, R_SIZE);
> + rtbytes = getnum(rtsize, &ropts, R_SIZE);
> if (rtbytes % XFS_MIN_BLOCKSIZE) {
> fprintf(stderr,
> _("illegal rt length %lld, not a multiple of %d\n"),
> @@ -2370,7 +2350,7 @@ _("warning: sparse inodes not supported without CRC support, disabled.\n"));
> if (rtextsize) {
> __uint64_t rtextbytes;
>
> - rtextbytes = getnum_checked(rtextsize, &ropts, R_EXTSIZE);
> + rtextbytes = getnum(rtextsize, &ropts, R_EXTSIZE);
> if (rtextbytes % blocksize) {
> fprintf(stderr,
> _("illegal rt extent size %lld, not a multiple of %d\n"),
> @@ -3466,8 +3446,8 @@ unknown(
>
> long long
> cvtnum(
> - unsigned int blocksize,
> - unsigned int sectorsize,
> + unsigned int blksize,
> + unsigned int sectsize,
> const char *s)
> {
> long long i;
> @@ -3480,9 +3460,9 @@ cvtnum(
> return i;
>
> if (*sp == 'b' && sp[1] == '\0')
> - return i * blocksize;
> + return i * blksize;
> if (*sp == 's' && sp[1] == '\0')
> - return i * sectorsize;
> + return i * sectsize;
>
> if (*sp == 'k' && sp[1] == '\0')
> return 1024LL * i;
> --
> 2.1.0
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
