"Hariom Verma via GitGitGadget" <gitgitgadget@xxxxxxxxx> 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? 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.