Re: [PATCH] mkfs: rename defaultval to flagval in opts

["Luis R. Rodriguez" <mcgrof@kernel.org>]

On Tue, Aug 29, 2017 at 08:36:53AM +1000, Dave Chinner wrote:
> On Fri, Aug 25, 2017 at 02:39:18AM +0200, Luis R. Rodriguez wrote:
> > On Fri, Aug 25, 2017 at 09:58:43AM +1000, Dave Chinner wrote:
> > > A clean design isolates the different functionality into
> > > self-contained modules and connects them with simple, easy to
> > > understand structures and minimal APIs.
> > > 
> > > The default behaviours we can modify are a much smaller subset of
> > > options than the CLI can modify. Hence if we define a structure that
> > > contains all the default options we can set, we also define the
> > > required config file contents.
> > 
> > I see, so the configuration file purpose is to be able to override
> > the smaller set of defaults, not supplement the CLI with all the
> > bells and whistles the CLI allows. I'll yield to you for this, it
> > sounds reasonable to me, however I suppose I should point out that
> > at least ext[2-4] do allow for CLI options, so its not clear to me
> > what that should implicate about mke2fs.conf.
> 
> I don't really care what mke2fs does with it's config file - it's
> got different constraints and behaviours, and likely different ways
> of setting defaults. If they have to set them via using CLI options
> in the confg file, then that's what they thought best.
> 
> I'm working on the model that if something can have a sane default
> defined, then it should be a known key/value pair in the config
> file which has a matching entry in the struct mkfs_default_params
> that is then used in the appropriate place in the code to apply that
> default. Using CLI option syntax for this just makes the config file
> format more complex and parsing it harder for no good reason.

Sounds good to me!

> > > IOWs, we have 4 clear modules and a set of data that is needed
> > > as inputs into each module:
> > > 
> > > Module				Connecting structure
> > > Setting defaults
> > > 				struct mkfs_default_params
> > > CLI parsing
> > > 				struct cli_params
> > > Validation + calculation
> > > 				struct mkfs_params
> > > On disk formatting
> > > 
> > > Note that there's nothing here that dictates how each module is
> > > implemented - all I've done is define modules and the data that
> > > needs to flow between the modules. I've simply applied a basic
> > > software engineering technique (SADT - structured analysis and
> > > design technique) and made a ruidmentary data flow diagram in my
> > > head to get to this.
> > 
> > Thanks, this helps. FWIW the way I split things out was
> > 
> > CLI  ------------
> >                   \
> >                    ---> params
> >                  /
> > config file ---
> > 
> > It would seem from what you are saying
> > 
> > 
> > CLI  ------------
> >                   \
> >                    ---> struct mkfs_default_params
> >                  /
> > config file ---
> > 
> > Is that right?
> 
> No. It's:
> 
> Build defaults --\
> 		  ---> mkfs_default_params -> CLI -> mkfs_params
> config file -----/

Perfect, thanks for the clarification. Full steam ahead!

  Luis