Re: [PATCH v2 1/4] config: split out config_parse_options

[Josh Steadmon <steadmon@google.com>]

On 2023.08.23 16:26, Junio C Hamano wrote:
> Josh Steadmon <steadmon@google.com> writes:
> 
> > From: Glen Choo <chooglen@google.com>
> >
> > "struct config_options" is a disjoint set of options options used by the
> > config parser (e.g. event listners) and options used by
> > config_with_options() (e.g. to handle includes, choose which config
> > files to parse).
> 
> There is some punctuation missing on the first line.  Perhaps an em-dash
> between "options---options" or something like that?

Cleaned up this and an additional typo in the description.

> > Split parser-only options into config_parse_options.
> >
> > Signed-off-by: Glen Choo <chooglen@google.com>
> > Signed-off-by: Josh Steadmon <steadmon@google.com>
> > ---
> >  bundle-uri.c |  2 +-
> >  config.c     | 14 +++++++-------
> >  config.h     | 37 ++++++++++++++++++++-----------------
> >  fsck.c       |  2 +-
> >  4 files changed, 29 insertions(+), 26 deletions(-)
> 
> > diff --git a/bundle-uri.c b/bundle-uri.c
> > index 4b5c49b93d..f93ca6a486 100644
> > --- a/bundle-uri.c
> > +++ b/bundle-uri.c
> > @@ -237,7 +237,7 @@ int bundle_uri_parse_config_format(const char *uri,
> >  				   struct bundle_list *list)
> >  {
> >  	int result;
> > -	struct config_options opts = {
> > +	struct config_parse_options opts = {
> >  		.error_action = CONFIG_ERROR_ERROR,
> >  	};
> 
> OK, and this one only needs the parse_options half, and presumably
> all hunks (other than the one that splits the struct into two in
> config.h) are about turning the users of config_options that only
> need config_parse_options half.
> 
> As we do not see any funny casts in the patch text, compilers should
> catch all questionable conversion in this step, if there were any.
> 
> OK.