| Message ID | 20181105063819.GA25864@sigill.intra.peff.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | parseopt fixes from -Wunused-parameters | expand |
Jeff King <peff@peff.net> writes: > The options callback for "git apply --no-include" is not ready to handle > the "unset" parameter, and as a result will segfault when it adds a NULL > argument to the include list (likewise for "--no-exclude"). > > In theory this might be used to clear the list, but since both > "--include" and "--exclude" add to the same list, it's not immediately > obvious what the semantics should be. Let's punt on that for now and > just disallow the broken options. Thanks. I agree with the conclusion to leave it to later outside this series to define what --no-(include|exclude) should do. I suspect something along the lines of Each element on the single list is marked as either include or exclude, and "--no-include" would remove the accumulated "include" entries in the list without touching any "exclude" elements. would be sufficiently clear and useful, perhaps. > > Signed-off-by: Jeff King <peff@peff.net> > --- > apply.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/apply.c b/apply.c > index 073d5f0451..d1ca6addeb 100644 > --- a/apply.c > +++ b/apply.c > @@ -4939,10 +4939,10 @@ int apply_parse_options(int argc, const char **argv, > struct option builtin_apply_options[] = { > { OPTION_CALLBACK, 0, "exclude", state, N_("path"), > N_("don't apply changes matching the given path"), > - 0, apply_option_parse_exclude }, > + PARSE_OPT_NONEG, apply_option_parse_exclude }, > { OPTION_CALLBACK, 0, "include", state, N_("path"), > N_("apply changes matching the given path"), > - 0, apply_option_parse_include }, > + PARSE_OPT_NONEG, apply_option_parse_include }, > { OPTION_CALLBACK, 'p', NULL, state, N_("num"), > N_("remove <num> leading slashes from traditional diff paths"), > 0, apply_option_parse_p },
diff --git a/apply.c b/apply.c index 073d5f0451..d1ca6addeb 100644 --- a/apply.c +++ b/apply.c @@ -4939,10 +4939,10 @@ int apply_parse_options(int argc, const char **argv, struct option builtin_apply_options[] = { { OPTION_CALLBACK, 0, "exclude", state, N_("path"), N_("don't apply changes matching the given path"), - 0, apply_option_parse_exclude }, + PARSE_OPT_NONEG, apply_option_parse_exclude }, { OPTION_CALLBACK, 0, "include", state, N_("path"), N_("apply changes matching the given path"), - 0, apply_option_parse_include }, + PARSE_OPT_NONEG, apply_option_parse_include }, { OPTION_CALLBACK, 'p', NULL, state, N_("num"), N_("remove <num> leading slashes from traditional diff paths"), 0, apply_option_parse_p },
The options callback for "git apply --no-include" is not ready to handle the "unset" parameter, and as a result will segfault when it adds a NULL argument to the include list (likewise for "--no-exclude"). In theory this might be used to clear the list, but since both "--include" and "--exclude" add to the same list, it's not immediately obvious what the semantics should be. Let's punt on that for now and just disallow the broken options. Signed-off-by: Jeff King <peff@peff.net> --- apply.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)