Re: [PATCH v2 1/4] config: split out config_parse_options
On 2023.08.23 16:26, Junio C Hamano wrote:
Show 11 quoted lines
> 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.
Show 33 quoted lines
> > 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.