| Message ID | 20170720092932.32580-8-jtulak@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 7/20/17 4:29 AM, Jan Tulak wrote: > Save the parsed values from users into the opts table. This will ensure that > the user values gets there and are validated, but we are not yet using them for > anything - the usage makes a lot of changes through the code, so it is better > if that is separated into smaller chunks. > > Signed-off-by: Jan Tulak <jtulak@redhat.com> This seems reasonable, but AFAICT nothing uses the set values yet, right? As such I'll probably hold off on merging it until somethig makes use of the result... i.e. merge it (and the prior patch) along with later patches which make use of the stored values. Also, questions below. > case I_PROJID32BIT: > sb_feat.projid16bit = > !getnum(value, &opts[OPT_I], > I_PROJID32BIT); > + set_conf_val(OPT_I, I_PROJID32BIT, > + sb_feat.projid16bit); > break; why isn't this just: sb_feat.projid16bit = parse_conf_val(OPT_I, I_PROJID32BIT, value) ? > @@ -1812,34 +1843,48 @@ main( > xi.logname = logfile; > ldflag = 1; > loginternal = 0; > + set_conf_val(OPT_L, L_NAME, 1); > + set_conf_val(OPT_L, L_DEV, 1); Hm, ok, maybe these subopts will collapse into one some day? > break; > case L_VERSION: > sb_feat.log_version = > - getnum(value, &opts[OPT_L], > - L_VERSION); > + parse_conf_val(OPT_L, > + L_VERSION, > + value); > lvflag = 1; > + set_conf_val(OPT_L, L_VERSION, > + sb_feat.log_version); > break; wait, didn't parse_conf_val already set a value into OPT_L, L_VERSION? > case M_FINOBT: > - sb_feat.finobt = getnum( > - value, &opts[OPT_M], M_FINOBT); > + sb_feat.finobt = > + parse_conf_val(OPT_M, M_FINOBT, > + value); > + set_conf_val(OPT_M, M_FINOBT, > + sb_feat.finobt); Same here, hasn't OPT_M, M_FINOBT's value already been set by the parse call? > @@ -1920,14 +1977,17 @@ main( > } else { > sb_feat.dir_version = > getnum(value, > - &opts[OPT_N], > - N_VERSION); > + &opts[OPT_N], > + N_VERSION); > } > nvflag = 1; > + set_conf_val(OPT_N, N_VERSION, > + sb_feat.dir_version); shouldn't this be in the else clause? We didn't necessarily set a version number, right? Also, should the ci state be stored as well? i.e. case N_VERSION: value = getstr(value, &nopts, N_VERSION); if (!strcasecmp(value, "ci")) { /* ASCII CI mode */ sb_feat.nci = true; /* is this in the opts table anywhere? */ } else { sb_feat.dir_version = parse_conf_val(OPT_N, N_VERSION, value); } nvflag = 1; break; > break; > case N_FTYPE: > - sb_feat.dirftype = getnum(value, > - &opts[OPT_N], N_FTYPE); > + sb_feat.dirftype = > + parse_conf_val(OPT_N, N_FTYPE, > + value); > break; > default: > unknown('n', value); > @@ -1957,25 +2017,30 @@ main( > > switch (getsubopt(&p, subopts, &value)) { > case R_EXTSIZE: > - rtextbytes = getnum(value, > - &opts[OPT_R], R_EXTSIZE); > + rtextbytes = parse_conf_val(OPT_R, > + R_EXTSIZE, > + value); > break; > case R_FILE: > - xi.risfile = getnum(value, > - &opts[OPT_R], R_FILE); > + xi.risfile = parse_conf_val(OPT_R, > + R_FILE, > + value); > break; > case R_NAME: > case R_DEV: > xi.rtname = getstr(value, &opts[OPT_R], > R_NAME); > + set_conf_val(OPT_R, R_NAME, 1); > + set_conf_val(OPT_R, R_DEV, 1); > break; > case R_SIZE: > - rtbytes = getnum(value, &opts[OPT_R], > - R_SIZE); > + rtbytes = parse_conf_val(OPT_R, R_SIZE, > + value); > break; > case R_NOALIGN: > - norsflag = getnum(value, &opts[OPT_R], > - R_NOALIGN); > + norsflag = parse_conf_val(OPT_R, > + R_NOALIGN, > + value); > break; > default: > unknown('r', value); > @@ -1996,12 +2061,14 @@ main( > conflict('s', subopts, > S_SECTSIZE, > S_SECTLOG); > - sectorlog = getnum(value, &opts[OPT_S], > - S_SECTLOG); > + sectorlog = parse_conf_val(OPT_S, > + S_SECTLOG, > + value); > lsectorlog = sectorlog; > sectorsize = 1 << sectorlog; > lsectorsize = sectorsize; > lslflag = slflag = 1; > + set_conf_val(OPT_S, S_LOG, sectorsize); Is this right? S_LOG is the log of the sectorsize, right, not the sector size itself. Do S_SIZE & S_SECTSIZE need to be stored as well? > break; > case S_SIZE: > case S_SECTSIZE: > @@ -2009,13 +2076,16 @@ main( > conflict('s', subopts, > S_SECTLOG, > S_SECTSIZE); > - sectorsize = getnum(value, > - &opts[OPT_S], S_SECTSIZE); > + sectorsize = parse_conf_val(OPT_S, > + S_SECTSIZE, > + value); > lsectorsize = sectorsize; > sectorlog = > libxfs_highbit32(sectorsize); > lsectorlog = sectorlog; > lssflag = ssflag = 1; > + set_conf_val(OPT_S, > + S_SIZE, sectorlog); same questions here - this looks wrong, and other values not stored, do they need to be? > break; > default: > unknown('s', value); > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 27/07/2017 01:53, Eric Sandeen wrote: > On 7/20/17 4:29 AM, Jan Tulak wrote: >> Save the parsed values from users into the opts table. This will ensure that >> the user values gets there and are validated, but we are not yet using them for >> anything - the usage makes a lot of changes through the code, so it is better >> if that is separated into smaller chunks. >> >> Signed-off-by: Jan Tulak <jtulak@redhat.com> > This seems reasonable, but AFAICT nothing uses the set values yet, > right? As such I'll probably hold off on merging it until somethig > makes use of the result... i.e. merge it (and the prior patch) > along with later patches which make use of the stored values. The second patchset I submitted just after this one uses it. I just split it, so this set doesn't have to wait if there are any issues in the second part. But if you would rather merge it together, I'm ok with it and if I will need to respin a whole set once more in a final version, I can put it together again. > > Also, questions below. > >> case I_PROJID32BIT: >> sb_feat.projid16bit = >> !getnum(value, &opts[OPT_I], >> I_PROJID32BIT); >> + set_conf_val(OPT_I, I_PROJID32BIT, >> + sb_feat.projid16bit); >> break; > why isn't this just: > > sb_feat.projid16bit = parse_conf_val(OPT_I, I_PROJID32BIT, value) ? Note the ! in the getnum assignment, we have two variables with opposite values (16/32 bit) - that's the reason why I used getnum. But there indeed is an issue with that - the set_conf_val shouldn't be the same value as the sb_feat. So, I'm changing it to: sb_feat.projid16bit = ! parse_conf_val(OPT_I, I_PROJID32BIT, value); > > >> @@ -1812,34 +1843,48 @@ main( >> xi.logname = logfile; >> ldflag = 1; >> loginternal = 0; >> + set_conf_val(OPT_L, L_NAME, 1); >> + set_conf_val(OPT_L, L_DEV, 1); > Hm, ok, maybe these subopts will collapse into one some day? It is on my (long) TODO. But it might be a change simple enough to do it ASAP. > >> break; >> case L_VERSION: >> sb_feat.log_version = >> - getnum(value, &opts[OPT_L], >> - L_VERSION); >> + parse_conf_val(OPT_L, >> + L_VERSION, >> + value); >> lvflag = 1; >> + set_conf_val(OPT_L, L_VERSION, >> + sb_feat.log_version); >> break; > wait, didn't parse_conf_val already set a value into OPT_L, L_VERSION? > > >> case M_FINOBT: >> - sb_feat.finobt = getnum( >> - value, &opts[OPT_M], M_FINOBT); >> + sb_feat.finobt = >> + parse_conf_val(OPT_M, M_FINOBT, >> + value); >> + set_conf_val(OPT_M, M_FINOBT, >> + sb_feat.finobt); > Same here, hasn't OPT_M, M_FINOBT's value already been set by the parse > call? Yes and yes. Removing. > >> @@ -1920,14 +1977,17 @@ main( >> } else { >> sb_feat.dir_version = >> getnum(value, >> - &opts[OPT_N], >> - N_VERSION); >> + &opts[OPT_N], >> + N_VERSION); >> } >> nvflag = 1; >> + set_conf_val(OPT_N, N_VERSION, >> + sb_feat.dir_version); > shouldn't this be in the else clause? We didn't necessarily set a > version number, right? Also, should the ci state be stored as well? > i.e. > > case N_VERSION: > value = getstr(value, &nopts, N_VERSION); > if (!strcasecmp(value, "ci")) { > /* ASCII CI mode */ > sb_feat.nci = true; > /* is this in the opts table anywhere? */ > } else { > sb_feat.dir_version = > parse_conf_val(OPT_N, > N_VERSION, > value); > } > nvflag = 1; > break; To be honest, I think this option is weird; it feels to me like two options merged into one. It has only two valid values: 2 and 'ci'. But 2 is a version, which could be any number, and ci is a modifier for the given version. I mean, it looks like it should be accepting "2 or 2ci", rather than "2 or ci". Because the ci is just a modifier and the version remains as 2. Or we can say that this option only decides ci/non-ci, and then the option would better to be renamed and turned to a flag. But either of those would be a visible change to the interface, so I tried to avoid it. sb_feat.dir_version seems to be used later in the code even when nci is set and the dir_version is currently always 2. Thus I put the assignment to happen either way, to avoid hardcoding one value at multiple places: we save whatever is the value of dir_version, either from user or from default (XFS_DFL_DIR_VERSION). At this moment, I'm not saving the ci flag anywhere. Once the opts can hold strings and not only numbers, it becomes no issue, but now I would have to add another option as a flag, or save something else than the version into opt (e.g. 1 or 3 or anything else than 2...) but that would bring its own issues. The way I did it (ignoring a value that we won't read from opts anytime soon anyway) seems the safest to me. > >> break; >> case N_FTYPE: >> - sb_feat.dirftype = getnum(value, >> - &opts[OPT_N], N_FTYPE); >> + sb_feat.dirftype = >> + parse_conf_val(OPT_N, N_FTYPE, >> + value); >> break; >> default: >> unknown('n', value); >> @@ -1957,25 +2017,30 @@ main( >> >> switch (getsubopt(&p, subopts, &value)) { >> case R_EXTSIZE: >> - rtextbytes = getnum(value, >> - &opts[OPT_R], R_EXTSIZE); >> + rtextbytes = parse_conf_val(OPT_R, >> + R_EXTSIZE, >> + value); >> break; >> case R_FILE: >> - xi.risfile = getnum(value, >> - &opts[OPT_R], R_FILE); >> + xi.risfile = parse_conf_val(OPT_R, >> + R_FILE, >> + value); >> break; >> case R_NAME: >> case R_DEV: >> xi.rtname = getstr(value, &opts[OPT_R], >> R_NAME); >> + set_conf_val(OPT_R, R_NAME, 1); >> + set_conf_val(OPT_R, R_DEV, 1); >> break; >> case R_SIZE: >> - rtbytes = getnum(value, &opts[OPT_R], >> - R_SIZE); >> + rtbytes = parse_conf_val(OPT_R, R_SIZE, >> + value); >> break; >> case R_NOALIGN: >> - norsflag = getnum(value, &opts[OPT_R], >> - R_NOALIGN); >> + norsflag = parse_conf_val(OPT_R, >> + R_NOALIGN, >> + value); >> break; >> default: >> unknown('r', value); >> @@ -1996,12 +2061,14 @@ main( >> conflict('s', subopts, >> S_SECTSIZE, >> S_SECTLOG); >> - sectorlog = getnum(value, &opts[OPT_S], >> - S_SECTLOG); >> + sectorlog = parse_conf_val(OPT_S, >> + S_SECTLOG, >> + value); >> lsectorlog = sectorlog; >> sectorsize = 1 << sectorlog; >> lsectorsize = sectorsize; >> lslflag = slflag = 1; >> + set_conf_val(OPT_S, S_LOG, sectorsize); > Is this right? S_LOG is the log of the sectorsize, right, not the sector size > itself. Do S_SIZE & S_SECTSIZE need to be stored as well? This appears to be an issue. I'll fix it. > >> break; >> case S_SIZE: >> case S_SECTSIZE: >> @@ -2009,13 +2076,16 @@ main( >> conflict('s', subopts, >> S_SECTLOG, >> S_SECTSIZE); >> - sectorsize = getnum(value, >> - &opts[OPT_S], S_SECTSIZE); >> + sectorsize = parse_conf_val(OPT_S, >> + S_SECTSIZE, >> + value); >> lsectorsize = sectorsize; >> sectorlog = >> libxfs_highbit32(sectorsize); >> lsectorlog = sectorlog; >> lssflag = ssflag = 1; >> + set_conf_val(OPT_S, >> + S_SIZE, sectorlog); > same questions here - this looks wrong, and other values not stored, do > they need to be? > >> break; >> default: >> unknown('s', value); >> Thanks, Jan -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c index e008b5a4..bf480efe 100644 --- a/mkfs/xfs_mkfs.c +++ b/mkfs/xfs_mkfs.c @@ -1616,16 +1616,19 @@ main(; switch (getsubopt(&p, subopts, &value)) { case B_LOG: - blocklog = getnum(value, &opts[OPT_B], - B_LOG); + blocklog = parse_conf_val(OPT_B, B_LOG, + value); blocksize = 1 << blocklog; blflag = 1; + set_conf_val(OPT_B, B_SIZE, blocksize); break; case B_SIZE: - blocksize = getnum(value, &opts[OPT_B], - B_SIZE); + blocksize = parse_conf_val(OPT_B, + B_SIZE, + value); blocklog = libxfs_highbit32(blocksize); bsflag = 1; + set_conf_val(OPT_B, B_LOG, blocklog); break; default: unknown('b', value); @@ -1641,76 +1644,92 @@ main( switch (getsubopt(&p, subopts, &value)) { case D_AGCOUNT: - agcount = getnum(value, &opts[OPT_D], - D_AGCOUNT); + agcount = parse_conf_val(OPT_D, + D_AGCOUNT, + value); daflag = 1; break; case D_AGSIZE: - agsize = getnum(value, &opts[OPT_D], - D_AGSIZE); + agsize = parse_conf_val(OPT_D, + D_AGSIZE, + value); dasize = 1; break; case D_FILE: - xi.disfile = getnum(value, - &opts[OPT_D], D_FILE); + xi.disfile = parse_conf_val(OPT_D, + D_FILE, + value); break; case D_NAME: xi.dname = getstr(value, &opts[OPT_D], D_NAME); + set_conf_val(OPT_D, D_NAME, 1); break; case D_SIZE: - dbytes = getnum(value, &opts[OPT_D], - D_SIZE); + dbytes = parse_conf_val(OPT_D, D_SIZE, + value); break; case D_SUNIT: - dsunit = getnum(value, &opts[OPT_D], - D_SUNIT); + dsunit = parse_conf_val(OPT_D, D_SUNIT, + value); break; case D_SWIDTH: - dswidth = getnum(value, &opts[OPT_D], - D_SWIDTH); + dswidth = parse_conf_val(OPT_D, + D_SWIDTH, + value); break; case D_SU: - dsu = getnum(value, &opts[OPT_D], - D_SU); + dsu = parse_conf_val(OPT_D, D_SU, + value); break; case D_SW: - dsw = getnum(value, &opts[OPT_D], - D_SW); + dsw = parse_conf_val(OPT_D, D_SW, + value); break; case D_NOALIGN: - nodsflag = getnum(value, &opts[OPT_D], - D_NOALIGN); + nodsflag = parse_conf_val(OPT_D, + D_NOALIGN, + value); break; case D_SECTLOG: - sectorlog = getnum(value, &opts[OPT_D], - D_SECTLOG); + sectorlog = parse_conf_val(OPT_D, + D_SECTLOG, + value); sectorsize = 1 << sectorlog; slflag = 1; + set_conf_val(OPT_D, D_SECTSIZE, + sectorsize); break; case D_SECTSIZE: - sectorsize = getnum(value, - &opts[OPT_D], D_SECTSIZE); + sectorsize = parse_conf_val(OPT_D, + D_SECTSIZE, + value); sectorlog = libxfs_highbit32(sectorsize); ssflag = 1; + set_conf_val(OPT_D, D_SECTLOG, + sectorlog); break; case D_RTINHERIT: - c = getnum(value, &opts[OPT_D], - D_RTINHERIT); + c = parse_conf_val(OPT_D, D_RTINHERIT, + value); if (c) fsx.fsx_xflags |= XFS_DIFLAG_RTINHERIT; break; case D_PROJINHERIT: - fsx.fsx_projid = getnum(value, - &opts[OPT_D], D_PROJINHERIT); + fsx.fsx_projid = + parse_conf_val(OPT_D, + D_PROJINHERIT, + value); fsx.fsx_xflags |= XFS_DIFLAG_PROJINHERIT; break; case D_EXTSZINHERIT: - fsx.fsx_extsize = getnum(value, - &opts[OPT_D], D_EXTSZINHERIT); + fsx.fsx_extsize = + parse_conf_val(OPT_D, + D_EXTSZINHERIT, + value); fsx.fsx_xflags |= XFS_DIFLAG_EXTSZINHERIT; break; @@ -1728,45 +1747,53 @@ main( switch (getsubopt(&p, subopts, &value)) { case I_ALIGN: - sb_feat.inode_align = getnum(value, - &opts[OPT_I], I_ALIGN); + sb_feat.inode_align = + parse_conf_val(OPT_I, I_ALIGN, + value); break; case I_LOG: - inodelog = getnum(value, &opts[OPT_I], - I_LOG); + inodelog = parse_conf_val(OPT_I, I_LOG, + value); isize = 1 << inodelog; ilflag = 1; + set_conf_val(OPT_I, I_SIZE, isize); break; case I_MAXPCT: - imaxpct = getnum(value, &opts[OPT_I], - I_MAXPCT); + imaxpct = parse_conf_val(OPT_I, + I_MAXPCT, + value); imflag = 1; break; case I_PERBLOCK: - inopblock = getnum(value, &opts[OPT_I], - I_PERBLOCK); + inopblock = parse_conf_val(OPT_I, + I_PERBLOCK, + value); ipflag = 1; break; case I_SIZE: - isize = getnum(value, &opts[OPT_I], - I_SIZE); + isize = parse_conf_val(OPT_I, I_SIZE, + value); inodelog = libxfs_highbit32(isize); isflag = 1; + set_conf_val(OPT_I, I_LOG, inodelog); break; case I_ATTR: sb_feat.attr_version = - getnum(value, &opts[OPT_I], - I_ATTR); + parse_conf_val(OPT_I, I_ATTR, + value); break; case I_PROJID32BIT: sb_feat.projid16bit = !getnum(value, &opts[OPT_I], I_PROJID32BIT); + set_conf_val(OPT_I, I_PROJID32BIT, + sb_feat.projid16bit); break; case I_SPINODES: - sb_feat.spinodes = getnum(value, - &opts[OPT_I], - I_SPINODES); + sb_feat.spinodes = + parse_conf_val(OPT_I, + I_SPINODES, + value); break; default: unknown('i', value); @@ -1782,27 +1809,31 @@ main( switch (getsubopt(&p, subopts, &value)) { case L_AGNUM: - logagno = getnum(value, &opts[OPT_L], - L_AGNUM); + logagno = parse_conf_val(OPT_L, + L_AGNUM, + value); laflag = 1; break; case L_FILE: - xi.lisfile = getnum(value, - &opts[OPT_L], L_FILE); + xi.lisfile = parse_conf_val(OPT_L, + L_FILE, + value); break; case L_INTERNAL: - loginternal = getnum(value, - &opts[OPT_L], L_INTERNAL); + loginternal = + parse_conf_val(OPT_L, + L_INTERNAL, + value); liflag = 1; break; case L_SU: - lsu = getnum(value, &opts[OPT_L], - L_SU); + lsu = parse_conf_val(OPT_L, L_SU, + value); lsuflag = 1; break; case L_SUNIT: - lsunit = getnum(value, &opts[OPT_L], - L_SUNIT); + lsunit = parse_conf_val(OPT_L, L_SUNIT, + value); lsunitflag = 1; break; case L_NAME: @@ -1812,34 +1843,48 @@ main( xi.logname = logfile; ldflag = 1; loginternal = 0; + set_conf_val(OPT_L, L_NAME, 1); + set_conf_val(OPT_L, L_DEV, 1); break; case L_VERSION: sb_feat.log_version = - getnum(value, &opts[OPT_L], - L_VERSION); + parse_conf_val(OPT_L, + L_VERSION, + value); lvflag = 1; + set_conf_val(OPT_L, L_VERSION, + sb_feat.log_version); break; case L_SIZE: - logbytes = getnum(value, - &opts[OPT_L], L_SIZE); + logbytes = parse_conf_val(OPT_L, + L_SIZE, + value); break; case L_SECTLOG: - lsectorlog = getnum(value, - &opts[OPT_L], L_SECTLOG); + lsectorlog = parse_conf_val(OPT_L, + L_SECTLOG, + value); lsectorsize = 1 << lsectorlog; lslflag = 1; + set_conf_val(OPT_L, L_SECTSIZE, + lsectorsize); break; case L_SECTSIZE: - lsectorsize = getnum(value, - &opts[OPT_L], L_SECTSIZE); + lsectorsize = + parse_conf_val(OPT_L, + L_SECTSIZE, + value); lsectorlog = libxfs_highbit32(lsectorsize); lssflag = 1; + set_conf_val(OPT_L, L_SECTLOG, + lsectorlog); break; case L_LAZYSBCNTR: sb_feat.lazy_sb_counters = - getnum(value, &opts[OPT_L], - L_LAZYSBCNTR); + parse_conf_val(OPT_L, + L_LAZYSBCNTR, + value); break; default: unknown('l', value); @@ -1861,29 +1906,35 @@ main( switch (getsubopt(&p, subopts, &value)) { case M_CRC: sb_feat.crcs_enabled = - getnum(value, &opts[OPT_M], - M_CRC); + parse_conf_val(OPT_M, M_CRC, + value); if (sb_feat.crcs_enabled) sb_feat.dirftype = true; break; case M_FINOBT: - sb_feat.finobt = getnum( - value, &opts[OPT_M], M_FINOBT); + sb_feat.finobt = + parse_conf_val(OPT_M, M_FINOBT, + value); + set_conf_val(OPT_M, M_FINOBT, + sb_feat.finobt); break; case M_UUID: if (!value || *value == '\0') reqval('m', subopts, M_UUID); if (platform_uuid_parse(value, &uuid)) illegal(optarg, "m uuid"); + set_conf_val(OPT_M, M_UUID, 1); break; case M_RMAPBT: - sb_feat.rmapbt = getnum( - value, &opts[OPT_M], M_RMAPBT); + sb_feat.rmapbt = + parse_conf_val(OPT_M, M_RMAPBT, + value); break; case M_REFLINK: sb_feat.reflink = - getnum(value, &opts[OPT_M], - M_REFLINK); + parse_conf_val(OPT_M, + M_REFLINK, + value); break; default: unknown('m', value); @@ -1899,17 +1950,23 @@ main( switch (getsubopt(&p, subopts, &value)) { case N_LOG: - dirblocklog = getnum(value, - &opts[OPT_N], N_LOG); + dirblocklog = parse_conf_val(OPT_N, + N_LOG, + value); dirblocksize = 1 << dirblocklog; nlflag = 1; + set_conf_val(OPT_N, N_SIZE, + dirblocksize); break; case N_SIZE: - dirblocksize = getnum(value, - &opts[OPT_N], N_SIZE); + dirblocksize = parse_conf_val(OPT_N, + N_SIZE, + value); dirblocklog = libxfs_highbit32(dirblocksize); nsflag = 1; + set_conf_val(OPT_N, N_LOG, + dirblocklog); break; case N_VERSION: value = getstr(value, &opts[OPT_N], @@ -1920,14 +1977,17 @@ main( } else { sb_feat.dir_version = getnum(value, - &opts[OPT_N], - N_VERSION); + &opts[OPT_N], + N_VERSION); } nvflag = 1; + set_conf_val(OPT_N, N_VERSION, + sb_feat.dir_version); break; case N_FTYPE: - sb_feat.dirftype = getnum(value, - &opts[OPT_N], N_FTYPE); + sb_feat.dirftype = + parse_conf_val(OPT_N, N_FTYPE, + value); break; default: unknown('n', value); @@ -1957,25 +2017,30 @@ main( switch (getsubopt(&p, subopts, &value)) { case R_EXTSIZE: - rtextbytes = getnum(value, - &opts[OPT_R], R_EXTSIZE); + rtextbytes = parse_conf_val(OPT_R, + R_EXTSIZE, + value); break; case R_FILE: - xi.risfile = getnum(value, - &opts[OPT_R], R_FILE); + xi.risfile = parse_conf_val(OPT_R, + R_FILE, + value); break; case R_NAME: case R_DEV: xi.rtname = getstr(value, &opts[OPT_R], R_NAME); + set_conf_val(OPT_R, R_NAME, 1); + set_conf_val(OPT_R, R_DEV, 1); break; case R_SIZE: - rtbytes = getnum(value, &opts[OPT_R], - R_SIZE); + rtbytes = parse_conf_val(OPT_R, R_SIZE, + value); break; case R_NOALIGN: - norsflag = getnum(value, &opts[OPT_R], - R_NOALIGN); + norsflag = parse_conf_val(OPT_R, + R_NOALIGN, + value); break; default: unknown('r', value); @@ -1996,12 +2061,14 @@ main( conflict('s', subopts, S_SECTSIZE, S_SECTLOG); - sectorlog = getnum(value, &opts[OPT_S], - S_SECTLOG); + sectorlog = parse_conf_val(OPT_S, + S_SECTLOG, + value); lsectorlog = sectorlog; sectorsize = 1 << sectorlog; lsectorsize = sectorsize; lslflag = slflag = 1; + set_conf_val(OPT_S, S_LOG, sectorsize); break; case S_SIZE: case S_SECTSIZE: @@ -2009,13 +2076,16 @@ main( conflict('s', subopts, S_SECTLOG, S_SECTSIZE); - sectorsize = getnum(value, - &opts[OPT_S], S_SECTSIZE); + sectorsize = parse_conf_val(OPT_S, + S_SECTSIZE, + value); lsectorsize = sectorsize; sectorlog = libxfs_highbit32(sectorsize); lsectorlog = sectorlog; lssflag = ssflag = 1; + set_conf_val(OPT_S, + S_SIZE, sectorlog); break; default: unknown('s', value);
Save the parsed values from users into the opts table. This will ensure that the user values gets there and are validated, but we are not yet using them for anything - the usage makes a lot of changes through the code, so it is better if that is separated into smaller chunks. Signed-off-by: Jan Tulak <jtulak@redhat.com> --- Edit: * updated description --- mkfs/xfs_mkfs.c | 260 +++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 165 insertions(+), 95 deletions(-)