Re: [PATCH v2 2/3] pretty.c: capture invalid trailer argument
- From
Hariom verma <hariom18599@gmail.com>
- Date
- Jan 30, 2021, 19:06 UTC
- Message-ID
- <CA+CkUQ_tyJMsxwopU=g-8keyK=Y1nzE1wgXnohQy3h8h_px4Nw@mail.gmail.com>
- In-Reply-To
- <xmqqo8h7qoci.fsf@gitster.c.googlers.com>
Hi,
On Sat, Jan 30, 2021 at 5:37 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 18 quoted lines
>
> "Hariom Verma via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > } else if (!match_placeholder_bool_arg(*arg, "only", arg, &opts->only_trailers) &&
> > !match_placeholder_bool_arg(*arg, "unfold", arg, &opts->unfold) &&
> > !match_placeholder_bool_arg(*arg, "keyonly", arg, &opts->key_only) &&
> > - !match_placeholder_bool_arg(*arg, "valueonly", arg, &opts->value_only))
> > - break;
> > + !match_placeholder_bool_arg(*arg, "valueonly", arg, &opts->value_only)) {
> > + size_t invalid_arg_len = strcspn(*arg, ",)");
> > + *invalid_arg = xstrndup(*arg, invalid_arg_len);
> > + return 1;
> > + }
> > }
> > return 0;
> > }
>
> Would the existing caller be OK with this change?Yes, I made sure of that.
Show 10 quoted lines
> It used to be that this parsing code simply _ignored_ unrecognised > trailer keyword because the very original just did a "break" and > fell though, but now because this returns non-zero, it causes the > caller rewritten in the patch [v2 1/3] to "goto trailer_out". > > It is not clear from your proposed log message if this would result > in behaviour change, and if so if that behaviour change was intended. > > I suspect that the behaviour change the code implements may be OK, > but the log message needs to discuss why it is OK.
I should have included that change in behaviour in the commit message. Will change that too.
Thanks, Hariom Verma