All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: "Jan Ťulák" <jtulak@redhat.com>
Cc: Dave Chinner <dchinner@redhat.com>, xfs@oss.sgi.com
Subject: Re: [PATCH 09/17] mkfs: use getnum_checked for all ranged parameters
Date: Fri, 26 Jun 2015 13:17:06 -0400	[thread overview]
Message-ID: <20150626171706.GE40750@bfoster.bfoster> (raw)
In-Reply-To: <1434711726-13092-10-git-send-email-jtulak@redhat.com>

On Fri, Jun 19, 2015 at 01:01:58PM +0200, Jan Ťulák wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Now that getnum_checked can handle min/max checking, use this for
> all parameters that take straight numbers and don't require unit
> conversions.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Jan Ťulák <jtulak@redhat.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  include/xfs_mkfs.h |   5 +-
>  mkfs/xfs_mkfs.c    | 152 +++++++++++++++++++++++++++--------------------------
>  2 files changed, 81 insertions(+), 76 deletions(-)
> 
> diff --git a/include/xfs_mkfs.h b/include/xfs_mkfs.h
> index 996b690..f5639b0 100644
> --- a/include/xfs_mkfs.h
> +++ b/include/xfs_mkfs.h
> @@ -42,8 +42,9 @@
>  
>  #define XFS_AG_BYTES(bblog)	((long long)BBSIZE << (bblog))
>  #define	XFS_AG_MIN_BYTES	((XFS_AG_BYTES(15)))	/* 16 MB */
> -#define XFS_AG_MIN_BLOCKS(blog)	((XFS_AG_BYTES(15)) >> (blog))
> -#define XFS_AG_MAX_BLOCKS(blog)	((XFS_AG_BYTES(31) - 1) >> (blog))
> +#define	XFS_AG_MAX_BYTES	((XFS_AG_BYTES(31)))	/* 1 TB */
> +#define XFS_AG_MIN_BLOCKS(blog)	(XFS_AG_MIN_BYTES >> (blog))
> +#define XFS_AG_MAX_BLOCKS(blog)	((XFS_AG_MAX_BYTES - 1) >> (blog))
>  
>  #define XFS_MAX_AGNUMBER	((xfs_agnumber_t)(NULLAGNUMBER - 1))
>  
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 3803779..332c491 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -125,6 +125,8 @@ const struct opt_params dopts = {
>  	},
>  	.subopt_params = {
>  		{ .index = D_AGCOUNT,
> +		  .minval = 1,
> +		  .maxval = XFS_MAX_AGNUMBER,
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = D_FILE,
> @@ -139,18 +141,26 @@ const struct opt_params dopts = {
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = D_SUNIT,
> +		  .minval = 0,
> +		  .maxval = UINT_MAX,
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = D_SWIDTH,
> +		  .minval = 0,
> +		  .maxval = UINT_MAX,
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = D_AGSIZE,
> +		  .minval = XFS_AG_MIN_BYTES,
> +		  .maxval = XFS_AG_MAX_BYTES,
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = D_SU,
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = D_SW,
> +		  .minval = 0,
> +		  .maxval = UINT_MAX,
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = D_SECTLOG,
> @@ -164,15 +174,23 @@ const struct opt_params dopts = {
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = D_NOALIGN,
> -		  .defaultval = SUBOPT_NEEDS_VAL,
> +		  .minval = 1,
> +		  .maxval = 1,
> +		  .defaultval = 1,
>  		},
>  		{ .index = D_RTINHERIT,
> -		  .defaultval = SUBOPT_NEEDS_VAL,
> +		  .minval = 1,
> +		  .maxval = 1,
> +		  .defaultval = 1,
>  		},
>  		{ .index = D_PROJINHERIT,
> +		  .minval = 0,
> +		  .maxval = UINT_MAX,
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = D_EXTSZINHERIT,
> +		  .minval = 0,
> +		  .maxval = UINT_MAX,
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  	},
> @@ -212,15 +230,23 @@ const struct opt_params iopts = {
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = I_MAXPCT,
> +		  .minval = 0,
> +		  .maxval = 100,
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = I_PERBLOCK,
> +		  .minval = XFS_MIN_INODE_PERBLOCK,
> +		  .maxval = XFS_MAX_BLOCKSIZE / XFS_DINODE_MIN_SIZE,
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = I_SIZE,
> +		  .minval = XFS_DINODE_MIN_SIZE,
> +		  .maxval = XFS_DINODE_MAX_SIZE,
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = I_ATTR,
> +		  .minval = 0,
> +		  .maxval = 2,
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = I_PROJID32BIT,
> @@ -262,6 +288,8 @@ const struct opt_params lopts = {
>  	},
>  	.subopt_params = {
>  		{ .index = L_AGNUM,
> +		  .minval = 0,
> +		  .maxval = UINT_MAX,
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = L_INTERNAL,
> @@ -273,9 +301,13 @@ const struct opt_params lopts = {
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = L_VERSION,
> +		  .minval = 1,
> +		  .maxval = 2,
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = L_SUNIT,
> +		  .minval = BTOBB(XLOG_MIN_RECORD_BSIZE),
> +		  .maxval = BTOBB(XLOG_MAX_RECORD_BSIZE),
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = L_SU,
> @@ -335,6 +367,8 @@ const struct opt_params nopts = {
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = N_VERSION,
> +		  .minval = 2,
> +		  .maxval = 2,
>  		  .defaultval = SUBOPT_NEEDS_VAL,
>  		},
>  		{ .index = N_FTYPE,
> @@ -1512,13 +1546,11 @@ main(
>  				switch (getsubopt(&p, (constpp)subopts,
>  						  &value)) {
>  				case D_AGCOUNT:
> -					if (!value || *value == '\0')
> -						reqval('d', subopts, D_AGCOUNT);
>  					if (daflag)
>  						respec('d', subopts, D_AGCOUNT);
> -					agcount = getnum(value, 0, 0, false);
> -					if ((__int64_t)agcount <= 0)
> -						illegal(value, "d agcount");
> +
> +					agcount = getnum_checked(value, &dopts,
> +								 D_AGCOUNT);
>  					daflag = 1;
>  					break;
>  				case D_AGSIZE:
> @@ -1553,28 +1585,22 @@ main(
>  					dsize = value;
>  					break;
>  				case D_SUNIT:
> -					if (!value || *value == '\0')
> -						reqval('d', subopts, D_SUNIT);
>  					if (dsunit)
>  						respec('d', subopts, D_SUNIT);
>  					if (nodsflag)
>  						conflict('d', subopts, D_NOALIGN,
>  							 D_SUNIT);
> -					dsunit = getnum(value, 0, 0, false);
> -					if (dsunit < 0)
> -						illegal(value, "d sunit");
> +					dsunit = getnum_checked(value, &dopts,
> +								 D_SUNIT);
>  					break;
>  				case D_SWIDTH:
> -					if (!value || *value == '\0')
> -						reqval('d', subopts, D_SWIDTH);
>  					if (dswidth)
>  						respec('d', subopts, D_SWIDTH);
>  					if (nodsflag)
>  						conflict('d', subopts, D_NOALIGN,
>  							 D_SWIDTH);
> -					dswidth = getnum(value, 0, 0, false);
> -					if (dswidth < 0)
> -						illegal(value, "d swidth");
> +					dswidth = getnum_checked(value, &dopts,
> +								 D_SWIDTH);
>  					break;
>  				case D_SU:
>  					if (!value || *value == '\0')
> @@ -1590,16 +1616,13 @@ main(
>  						illegal(value, "d su");
>  					break;
>  				case D_SW:
> -					if (!value || *value == '\0')
> -						reqval('d', subopts, D_SW);
>  					if (dsw)
>  						respec('d', subopts, D_SW);
>  					if (nodsflag)
>  						conflict('d', subopts, D_NOALIGN,
>  							 D_SW);
> -					dsw = getnum(value, 0, 0, false);
> -					if (dsw < 0)
> -						illegal(value, "d sw");
> +					dsw = getnum_checked(value, &dopts,
> +								 D_SW);
>  					break;
>  				case D_NOALIGN:
>  					if (dsu)
> @@ -1645,21 +1668,22 @@ main(
>  					ssflag = 1;
>  					break;
>  				case D_RTINHERIT:
> -					fsx.fsx_xflags |= \
> -						XFS_DIFLAG_RTINHERIT;
> +					c = getnum_checked(value, &dopts,
> +							   D_RTINHERIT);
> +					if (c)
> +						fsx.fsx_xflags |=
> +							XFS_DIFLAG_RTINHERIT;
>  					break;
>  				case D_PROJINHERIT:
> -					if (!value || *value == '\0')
> -						reqval('d', subopts, D_PROJINHERIT);
> -					fsx.fsx_projid = atoi(value);
> -					fsx.fsx_xflags |= \
> +					fsx.fsx_projid = getnum_checked(value,
> +							&dopts, D_PROJINHERIT);
> +					fsx.fsx_xflags |=
>  						XFS_DIFLAG_PROJINHERIT;
>  					break;
>  				case D_EXTSZINHERIT:
> -					if (!value || *value == '\0')
> -						reqval('d', subopts, D_EXTSZINHERIT);
> -					fsx.fsx_extsize = atoi(value);
> -					fsx.fsx_xflags |= \
> +					fsx.fsx_extsize = getnum_checked(value,
> +							&dopts, D_EXTSZINHERIT);
> +					fsx.fsx_xflags |=
>  						XFS_DIFLAG_EXTSZINHERIT;
>  					break;
>  				default:
> @@ -1694,18 +1718,13 @@ main(
>  					ilflag = 1;
>  					break;
>  				case I_MAXPCT:
> -					if (!value || *value == '\0')
> -						reqval('i', subopts, I_MAXPCT);
>  					if (imflag)
>  						respec('i', subopts, I_MAXPCT);
> -					imaxpct = getnum(value, 0, 0, false);
> -					if (imaxpct < 0 || imaxpct > 100)
> -						illegal(value, "i maxpct");
> +					imaxpct = getnum_checked(value, &iopts,
> +								 I_MAXPCT);
>  					imflag = 1;
>  					break;
>  				case I_PERBLOCK:
> -					if (!value || *value == '\0')
> -						reqval('i', subopts, I_PERBLOCK);
>  					if (ilflag)
>  						conflict('i', subopts, I_LOG,
>  							 I_PERBLOCK);
> @@ -1714,16 +1733,13 @@ main(
>  					if (isflag)
>  						conflict('i', subopts, I_SIZE,
>  							 I_PERBLOCK);
> -					inopblock = getnum(value, 0, 0, false);
> -					if (inopblock <
> -						XFS_MIN_INODE_PERBLOCK ||
> -					    !ispow2(inopblock))
> +					inopblock = getnum_checked(value, &iopts,
> +								   I_PERBLOCK);
> +					if (!ispow2(inopblock))
>  						illegal(value, "i perblock");
>  					ipflag = 1;
>  					break;
>  				case I_SIZE:
> -					if (!value || *value == '\0')
> -						reqval('i', subopts, I_SIZE);
>  					if (ilflag)
>  						conflict('i', subopts, I_LOG,
>  							 I_SIZE);
> @@ -1732,19 +1748,17 @@ main(
>  							 I_SIZE);
>  					if (isflag)
>  						respec('i', subopts, I_SIZE);
> -					isize = getnum(value, 0, 0, true);
> -					if (isize <= 0 || !ispow2(isize))
> +					isize = getnum_checked(value, &iopts,
> +							       I_SIZE);
> +					if (!ispow2(isize))
>  						illegal(value, "i size");
>  					inodelog = libxfs_highbit32(isize);
>  					isflag = 1;
>  					break;
>  				case I_ATTR:
> -					if (!value || *value == '\0')
> -						reqval('i', subopts, I_ATTR);
> -					c = getnum(value, 0, 0, false);
> -					if (c < 0 || c > 2)
> -						illegal(value, "i attr");
> -					sb_feat.attr_version = c;
> +					sb_feat.attr_version =
> +						getnum_checked(value, &iopts,
> +							       I_ATTR);
>  					break;
>  				case I_PROJID32BIT:
>  					sb_feat.projid16bit =
> @@ -1772,15 +1786,12 @@ main(
>  				switch (getsubopt(&p, (constpp)subopts,
>  						  &value)) {
>  				case L_AGNUM:
> -					if (!value || *value == '\0')
> -						reqval('l', subopts, L_AGNUM);
>  					if (laflag)
>  						respec('l', subopts, L_AGNUM);
>  					if (ldflag)
>  						conflict('l', subopts, L_AGNUM, L_DEV);
> -					logagno = getnum(value, 0, 0, false);
> -					if (logagno < 0)
> -						illegal(value, "l agno");
> +					logagno = getnum_checked(value, &lopts,
> +								 L_AGNUM);
>  					laflag = 1;
>  					break;
>  				case L_FILE:
> @@ -1817,13 +1828,10 @@ main(
>  					lsuflag = 1;
>  					break;
>  				case L_SUNIT:
> -					if (!value || *value == '\0')
> -						reqval('l', subopts, L_SUNIT);
>  					if (lsunit)
>  						respec('l', subopts, L_SUNIT);
> -					lsunit = getnum(value, 0, 0, false);
> -					if (lsunit < 0)
> -						illegal(value, "l sunit");
> +					lsunit = getnum_checked(value, &lopts,
> +								 L_SUNIT);
>  					lsunitflag = 1;
>  					break;
>  				case L_NAME:
> @@ -1842,14 +1850,11 @@ main(
>  					xi.logname = value;
>  					break;
>  				case L_VERSION:
> -					if (!value || *value == '\0')
> -						reqval('l', subopts, L_VERSION);
>  					if (lvflag)
>  						respec('l', subopts, L_VERSION);
> -					c = getnum(value, 0, 0, false);
> -					if (c < 1 || c > 2)
> -						illegal(value, "l version");
> -					sb_feat.log_version = c;
> +					sb_feat.log_version =
> +						getnum_checked(value, &lopts,
> +							       L_VERSION);
>  					lvflag = 1;
>  					break;
>  				case L_SIZE:
> @@ -1981,11 +1986,10 @@ _("cannot specify both -m crc=1 and -n ftype\n"));
>  						/* ASCII CI mode */
>  						sb_feat.nci = true;
>  					} else {
> -						c = getnum(value, 0, 0, false);
> -						if (c != 2)
> -							illegal(value,
> -								"n version");
> -						sb_feat.dir_version = c;
> +						sb_feat.dir_version =
> +							getnum_checked(value,
> +								&nopts,
> +								N_VERSION);
>  					}
>  					nvflag = 1;
>  					break;
> -- 
> 2.1.0
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2015-06-26 17:17 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-19 11:01 [PATCH 00/17] mkfs: sanitise input parameters Jan Ťulák
2015-06-19 11:01 ` [PATCH 01/17] xfsprogs: use common code for multi-disk detection Jan Ťulák
2015-06-19 11:10   ` Christoph Hellwig
2015-06-19 11:51     ` Jan Tulak
2015-06-25 19:37   ` Brian Foster
2015-07-02 12:47     ` Jan Tulak
2015-07-02 14:14       ` Brian Foster
2015-07-02 23:05         ` Dave Chinner
2015-07-03 13:22           ` Brian Foster
2015-07-08 16:14           ` Jan Tulak
2015-07-09  0:45             ` Dave Chinner
2015-07-09  8:24               ` Jan Tulak
2015-07-03 10:06         ` Jan Tulak
2015-06-19 11:01 ` [PATCH 02/17] mkfs: sanitise ftype parameter values Jan Ťulák
2015-06-25 19:37   ` Brian Foster
2015-06-19 11:01 ` [PATCH 03/17] mkfs: Sanitise the superblock feature macros Jan Ťulák
2015-06-25 19:38   ` Brian Foster
2015-07-03  9:53     ` Jan Tulak
2015-07-03 13:24       ` Brian Foster
2015-06-19 11:01 ` [PATCH 04/17] mkfs: validate all input values Jan Ťulák
2015-06-25 19:38   ` Brian Foster
2015-06-19 11:01 ` [PATCH 05/17] mkfs: factor boolean option parsing Jan Ťulák
2015-06-25 19:38   ` Brian Foster
2015-06-19 11:01 ` [PATCH 06/17] mkfs: validate logarithmic parameters sanely Jan Ťulák
2015-06-26 17:16   ` Brian Foster
2015-06-19 11:01 ` [PATCH 07/17] mkfs: structify input parameter passing Jan Ťulák
2015-06-26 17:16   ` Brian Foster
2015-06-19 11:01 ` [PATCH 08/17] mkfs: getbool is redundant Jan Ťulák
2015-06-26 17:17   ` Brian Foster
2015-06-30  1:32     ` Dave Chinner
2015-06-19 11:01 ` [PATCH 09/17] mkfs: use getnum_checked for all ranged parameters Jan Ťulák
2015-06-26 17:17   ` Brian Foster [this message]
2015-06-19 11:01 ` [PATCH 10/17] mkfs: add respecification detection to generic parsing Jan Ťulák
2015-06-26 17:17   ` Brian Foster
2015-06-19 11:02 ` [PATCH 11/17] mkfs: table based parsing for converted parameters Jan Ťulák
2015-06-26 17:17   ` Brian Foster
2015-06-19 11:02 ` [PATCH 12/17] mkfs: merge getnum Jan Ťulák
2015-06-26 17:17   ` Brian Foster
2015-06-19 11:02 ` [PATCH 13/17] mkfs: encode conflicts into parsing table Jan Ťulák
2015-06-26 17:17   ` Brian Foster
2015-06-30  3:57     ` Dave Chinner
2015-06-30 11:27       ` Brian Foster
2015-07-01  8:30         ` Jan Tulak
2015-06-19 11:02 ` [PATCH 14/17] mkfs: add string options to generic parsing Jan Ťulák
2015-06-26 19:32   ` Brian Foster
2015-06-19 11:02 ` [PATCH 15/17] mkfs: don't treat files as though they are block devices Jan Ťulák
2015-06-26 19:32   ` Brian Foster
2015-06-19 11:02 ` [PATCH 16/17] mkfs fix: handling of files Jan Ťulák
2015-06-26 19:32   ` Brian Foster
2015-06-19 11:02 ` [PATCH 17/17] mkfs: move spinodes crc check Jan Ťulák
2015-06-26 19:32   ` Brian Foster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150626171706.GE40750@bfoster.bfoster \
    --to=bfoster@redhat.com \
    --cc=dchinner@redhat.com \
    --cc=jtulak@redhat.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.