| Message ID | 20181127165211.24763-2-pclouds@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce new commands switch-branch and checkout-files | expand |
On Tue, Nov 27, 2018 at 8:53 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> 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?
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> 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 <pclouds@gmail.com> > --- > 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++;
On Tue, Nov 27, 2018 at 8:44 PM Stefan Beller <sbeller@google.com> wrote: > > On Tue, Nov 27, 2018 at 8:53 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> 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.
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++;
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); Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- parse-options-cb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)