On Tue, Apr 25, 2017 at 10:16:13AM +0200, Jan Tulak wrote:
> On Tue, Apr 25, 2017 at 7:19 AM, Luis R. Rodriguez <mcgrof@xxxxxxxxxx> wrote:
> > On Sun, Apr 23, 2017 at 08:54:58PM +0200, Jan Tulak wrote:
> >> Save the parsed values from users into the opts table.
> >>
> >> Signed-off-by: Jan Tulak <jtulak@xxxxxxxxxx>
> >> ---
> >> mkfs/xfs_mkfs.c | 260 +++++++++++++++++++++++++++++++++++---------------------
> >> 1 file changed, 165 insertions(+), 95 deletions(-)
> >>
> >> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> >> index 4caf93c..362c9b4 100644
> >> --- a/mkfs/xfs_mkfs.c
> >> +++ b/mkfs/xfs_mkfs.c
> >> @@ -1636,16 +1636,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);
> >
> > This still means that users of parse_conf_val() must copy the same code
> > in each case in the above to handle collateral values: if blocklog was
> > passed we update blocklog *and* blocksize *and* bsflag. Likewise if blocksize
> > was passed we must update blocksize *and* blocklog *and* blflag. What I had
> > done on the mkfs.xfs.conf series is put all this functionality into a helper,
> > and to do this move all needed vars into a struct. You are moving all possible
> > user params to a option specific table, however the collateral parameters are
> > kept inside main().
> >
> > I'll need to do the same again: move the logic above to a helper and move
> > collateral parameters to a struct, or this could be done in your series by
> > having parse_conf_val() handle all the needed logic provided say an extra
> > struct param is passed -- or though some other means.
> >
> > Duplicating the functionality I'm sure is not desirable, how we handle this
> > subjective so I'll leave it up to you to advise how you'd like to proceed,
> > but let me know if the above consideration is clear.
> >
> > Luis
>
> This is something I want to do, but let's get there step by step.
> First make the table working, then remove pieces from the main. If I
> try to do everything at once, it won't be ever finished. ;-)
Sure, it was not clear if this work was just it, thanks for the clarification.
Will review further patches with this in mind!
Luis
--
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
