Re: [PATCH v2 2/3] pretty.c: capture invalid trailer argument
flat view
From: Junio C Hamano <hidden>
Date: 2021-01-30 00:08:10
"Hariom Verma via GitGitGadget" [off-list ref] writes:
quoted hunk
} 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? 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. Thanks.