Re: [PATCH v2 1/7] parse-options: allow parse_options_concat(NULL, options)
On Tue, Nov 27, 2018 at 8:44 PM Stefan Beller wrote: > > On Tue, Nov 27, 2018 at 8:53 AM Nguyễn Thái Ngọc Duy > wrote: > > > > There is currently no caller that calls this function with "a" being > > NULL. But it will be introduced shortly. It is used to construct the > > option array from scratch, e.g. > > > >struct parse_options opts = NULL; > >opts = parse_options_concat(opts, opts_1); > >opts = parse_options_concat(opts, opts_2); > > While this addresses the immediate needs, I'd prefer to think > about the API exposure of parse_options_concat, > (related: do we want to have docs in its header file?) > and I'd recommend to make it symmetrical, i.e. > allow the second argument to also be NULL? I'll just drop this patch. There's a better way to do the same without adding special handling like this. -- Duy
Re: [PATCH v2 1/7] parse-options: allow parse_options_concat(NULL, options)
Nguyễn Thái Ngọc Duy writes: > There is currently no caller that calls this function with "a" being > NULL. But it will be introduced shortly. It is used to construct the > option array from scratch, e.g. > >struct parse_options opts = NULL; Missing asterisk somewhere? >opts = parse_options_concat(opts, opts_1); >opts = parse_options_concat(opts, opts_2); > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > parse-options-cb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/parse-options-cb.c b/parse-options-cb.c > index 8c9edce52f..c609d52926 100644 > --- a/parse-options-cb.c > +++ b/parse-options-cb.c > @@ -126,7 +126,7 @@ struct option *parse_options_concat(struct option *a, > struct option *b) > struct option *ret; > size_t i, a_len = 0, b_len = 0; > > - for (i = 0; a[i].type != OPTION_END; i++) > + for (i = 0; a && a[i].type != OPTION_END; i++) > a_len++; > for (i = 0; b[i].type != OPTION_END; i++) > b_len++;
Re: [PATCH v2 1/7] parse-options: allow parse_options_concat(NULL, options)
On Tue, Nov 27, 2018 at 8:53 AM Nguyễn Thái Ngọc Duy wrote:
>
> There is currently no caller that calls this function with "a" being
> NULL. But it will be introduced shortly. It is used to construct the
> option array from scratch, e.g.
>
>struct parse_options opts = NULL;
>opts = parse_options_concat(opts, opts_1);
>opts = parse_options_concat(opts, opts_2);
While this addresses the immediate needs, I'd prefer to think
about the API exposure of parse_options_concat,
(related: do we want to have docs in its header file?)
and I'd recommend to make it symmetrical, i.e.
allow the second argument to also be NULL?
In the example given here, you'd just short it to
struct parse_options opts = opts_1;
opts = parse_options_concat(opts, opts_2);
if not for this patch. Are opts_{1,2} ever be NULL?
