On Thu, Jul 20, 2017 at 5:54 PM, Darrick J. Wong
<darrick.wong@xxxxxxxxxx> wrote:
> On Thu, Jul 20, 2017 at 11:29:28AM +0200, Jan Tulak wrote:
>> Some options loaded a number as a string with getstr and converted it to
>> number with getnum later in the code, without any reason for this
>> approach. (They were probably forgotten in some past cleaning.)
>>
>> This patch changes them to skip the string and use getnum directly in
>> the main option-parsing loop, as do all the other numerical options.
>> And as we now have two variables of the same type for the same value,
>> merge them together. (e.g. former string dsize and number dbytes).
>>
>> Signed-off-by: Jan Tulak <jtulak@xxxxxxxxxx>
>> Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>
>> Reviewed-by: Luis R. Rodriguez <mcgrof@xxxxxxxxxx>
>>
>> ---
>> In reply to Eric's comment, so it doesn't pop up again:
>> This patch has to be applied after "mkfs: Save raw user input ...",
>> because otherwise we would temporary lose the access to strings
>> with user-given values and thus wouldn't be able to report them in
>> case of any issue.
>> ---
>> mkfs/xfs_mkfs.c | 90 ++++++++++++++++++++++++++-------------------------------
>> 1 file changed, 41 insertions(+), 49 deletions(-)
>>
>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>> index f2f6468e..127f14c3 100644
>> --- a/mkfs/xfs_mkfs.c
>> +++ b/mkfs/xfs_mkfs.c
>> @@ -1345,6 +1345,7 @@ getnum(
>> long long c;
>>
>> check_opt(opts, index, false);
>> + set_conf_raw(opts, index, str);
>> /* empty strings might just return a default value */
>> if (!str || *str == '\0') {
>> if (sp->flagval == SUBOPT_NEEDS_VAL)
>> @@ -1432,12 +1433,12 @@ main(
>> char *dfile;
>> int dirblocklog;
>> int dirblocksize;
>> - char *dsize;
>> + int dbytes;
>> int dsu;
>> int dsw;
>> int dsunit;
>> int dswidth;
>> - int force_overwrite;
>> + bool force_overwrite;
>> struct fsxattr fsx;
>> int ilflag;
>> int imaxpct;
>> @@ -1456,7 +1457,7 @@ main(
>> xfs_rfsblock_t logblocks;
>> char *logfile;
>> int loginternal;
>> - char *logsize;
>> + int logbytes;
>> xfs_fsblock_t logstart;
>> int lvflag;
>> int lsflag;
>> @@ -1485,11 +1486,11 @@ main(
>> char *protostring;
>> int qflag;
>> xfs_rfsblock_t rtblocks;
>> + int rtbytes;
>> xfs_extlen_t rtextblocks;
>> xfs_rtblock_t rtextents;
>> - char *rtextsize;
>> + int rtextbytes;
>> char *rtfile;
>> - char *rtsize;
>> xfs_sb_t *sbp;
>> int sectorlog;
>> __uint64_t sector_mask;
>> @@ -1537,7 +1538,8 @@ main(
>> qflag = 0;
>> imaxpct = inodelog = inopblock = isize = 0;
>> dfile = logfile = rtfile = NULL;
>> - dsize = logsize = rtsize = rtextsize = protofile = NULL;
>> + protofile = NULL;
>> + rtbytes = rtextbytes = logbytes = dbytes = 0;
>> dsu = dsw = dsunit = dswidth = lalign = lsu = lsunit = 0;
>> nodsflag = norsflag = 0;
>> force_overwrite = 0;
>> @@ -1601,7 +1603,7 @@ main(
>> xi.dname = getstr(value, &dopts, D_NAME);
>> break;
>> case D_SIZE:
>> - dsize = getstr(value, &dopts, D_SIZE);
>> + dbytes = getnum(value, &dopts, D_SIZE);
>> break;
>> case D_SUNIT:
>> dsunit = getnum(value, &dopts, D_SUNIT);
>> @@ -1746,7 +1748,7 @@ main(
>> lvflag = 1;
>> break;
>> case L_SIZE:
>> - logsize = getstr(value, &lopts, L_SIZE);
>> + logbytes = getnum(value, &lopts, L_SIZE);
>> break;
>> case L_SECTLOG:
>> lsectorlog = getnum(value, &lopts,
>> @@ -1875,8 +1877,7 @@ main(
>>
>> switch (getsubopt(&p, subopts, &value)) {
>> case R_EXTSIZE:
>> - rtextsize = getstr(value, &ropts,
>> - R_EXTSIZE);
>> + rtextbytes = getnum(value, &ropts, R_EXTSIZE);
>> break;
>> case R_FILE:
>> xi.risfile = getnum(value, &ropts,
>> @@ -1888,7 +1889,7 @@ main(
>> R_NAME);
>> break;
>> case R_SIZE:
>> - rtsize = getstr(value, &ropts, R_SIZE);
>> + rtbytes = getnum(value, &ropts, R_SIZE);
>> break;
>> case R_NOALIGN:
>> norsflag = getnum(value, &ropts,
>> @@ -1991,14 +1992,14 @@ _("Minimum block size for CRC enabled filesystems is %d bytes.\n"),
>> * sector size mismatches between the new filesystem and the underlying
>> * host filesystem.
>> */
>> - check_device_type(dfile, &xi.disfile, !dsize, !dfile,
>> + check_device_type(dfile, &xi.disfile, !dbytes, !dfile,
>> Nflag ? NULL : &xi.dcreat, force_overwrite, "d");
>> if (!loginternal)
>> - check_device_type(xi.logname, &xi.lisfile, !logsize, !xi.logname,
>> - Nflag ? NULL : &xi.lcreat,
>> + check_device_type(xi.logname, &xi.lisfile, !logbytes,
>> + !xi.logname, Nflag ? NULL : &xi.lcreat,
>> force_overwrite, "l");
>> if (xi.rtname)
>> - check_device_type(xi.rtname, &xi.risfile, !rtsize, !xi.rtname,
>> + check_device_type(xi.rtname, &xi.risfile, !rtbytes, !xi.rtname,
>> Nflag ? NULL : &xi.rcreat,
>> force_overwrite, "r");
>> if (xi.disfile || xi.lisfile || xi.risfile)
>> @@ -2179,10 +2180,7 @@ _("rmapbt not supported with realtime devices\n"));
>> }
>>
>>
>> - if (dsize) {
>> - __uint64_t dbytes;
>> -
>> - dbytes = getnum(dsize, &dopts, D_SIZE);
>> + if (dbytes) {
>
> Why has dbytes been demoted from uint64_t to int? This eliminates the
> ability to -d size=8G, right?
>
Ah, thanks for spotting it, I will check the other variables as well.
Jan
--
Jan Tulak
jtulak@xxxxxxxxxx / jan@xxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
