"Hariom Verma via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > + size_t invalid_arg_len = strcspn(*arg, ",)"); > + *invalid_arg = xstrndup(*arg, invalid_arg_len); > + return 1; How about doing this only when invalid_arg is not NULL, i.e. } else if (!match_placeholder_bool_arg(....) && ... !match_placeholder_bool_arg(....)) { if (invalid_arg) { size_t len = strcspn(*arg, ",)"); *invalid_arg = xstrndup(*arg, len); } return -1; } Note that I just used 'len'; when the scope of a variable is so short, it is clear the length of what thing it refers to from the context, and there is no point in using a variable name that long. In any case, by doing so, the callers that are not interested in the report can just pass NULL, which means ... > @@ -1464,12 +1472,13 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ > struct strbuf sepbuf = STRBUF_INIT; > struct strbuf kvsepbuf = STRBUF_INIT; > size_t ret = 0; > + char *unused = NULL; ... this will become unneeded, and ... > opts.no_divider = 1; > > if (*arg == ':') { > arg++; > - if (format_set_trailers_options(&opts, &filter_list, &sepbuf, &kvsepbuf, &arg)) > + if (format_set_trailers_options(&opts, &filter_list, &sepbuf, &kvsepbuf, &arg, &unused)) > goto trailer_out; ... this will pass NULL, and ... > } > if (*arg == ')') { > @@ -1479,6 +1488,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ > trailer_out: > string_list_clear(&filter_list, 0); > strbuf_release(&sepbuf); > + free((char *)unused); ... this will become unneeded. > return ret; > } > > diff --git a/pretty.h b/pretty.h > index 7369cf7e148..d902cdd70a9 100644 > --- a/pretty.h > +++ b/pretty.h > @@ -151,6 +151,7 @@ int format_set_trailers_options(struct process_trailer_options *opts, > struct string_list *filter_list, > struct strbuf *sepbuf, > struct strbuf *kvsepbuf, > - const char **arg); > + const char **arg, > + char **invalid_arg); > > #endif /* PRETTY_H */