Re: [PATCH 6/7] trailer: handle NULL value when parsing trailer-specific config
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Dec 7, 2023, 08:14 UTC
- Message-ID
- <ZXF-_bS6upwHIWMh@tanuki>
- In-Reply-To
- <20231207071132.GF1276005@coredump.intra.peff.net>
On Thu, Dec 07, 2023 at 02:11:32AM -0500, Jeff King wrote:
Show 47 quoted lines
> When parsing the "key", "command", and "cmd" trailer config, we just
> make a copy of the value string. If we see an implicit bool like:
>
> [trailer "foo"]
> key
>
> we'll segfault trying to copy a NULL pointer. We can fix this with the
> usual config_error_nonbool() check.
>
> I split this out from the other vanilla cases, because at first glance
> it looks like a better fix here would be to move the NULL check out of
> the switch statement. But it would change the behavior of other keys
> like trailer.*.ifExists, where an implicit bool is interpreted as
> EXISTS_DEFAULT.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> trailer.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/trailer.c b/trailer.c
> index b0e2ec224a..e4b08ed267 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -553,16 +553,22 @@ static int git_trailer_config(const char *conf_key, const char *value,
> case TRAILER_KEY:
> if (conf->key)
> warning(_("more than one %s"), conf_key);
> + if (!value)
> + return config_error_nonbool(conf_key);
> conf->key = xstrdup(value);
> break;
> case TRAILER_COMMAND:
> if (conf->command)
> warning(_("more than one %s"), conf_key);
> + if (!value)
> + return config_error_nonbool(conf_key);
> conf->command = xstrdup(value);
> break;
> case TRAILER_CMD:
> if (conf->cmd)
> warning(_("more than one %s"), conf_key);
> + if (!value)
> + return config_error_nonbool(conf_key);
> conf->cmd = xstrdup(value);
> break;
> case TRAILER_WHERE:For the other cases we only generate warnings for unknown values, but return successfully. Should we do the same here?
Patrick