Re: [PATCH v5 4/8] t/helper/test-config: check argc before accessing argv

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sep 2, 2020, 07:18 UTC
Message-ID: <CAPig+cT7bO97kWgQCftetR_4i_eTU4FsGejkdkY+cbd5K78oLA@mail.gmail.com>
In-Reply-To: <0750191342754bcca398c6fdad522616b0f3fbc3.1599026986.git.matheus.bernardino@usp.br>

On Wed, Sep 2, 2020 at 2:18 AM Matheus Tavares <matheus.bernardino@usp.br> wrote:

Show 12 quoted lines

> Check that we have the expected argc in 'configset_get_value' and
> 'configset_get_value_multi' before trying to access argv elements.
>
> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
> ---
> diff --git a/t/helper/test-config.c b/t/helper/test-config.c
> @@ -138,7 +138,7 @@ int cmd__config(int argc, const char **argv)
> -       } else if (!strcmp(argv[1], "configset_get_value")) {
> +       } else if (argc >= 3 && !strcmp(argv[1], "configset_get_value")) {
>                 for (i = 3; i < argc; i++) {
> @@ -156,7 +156,7 @@ int cmd__config(int argc, const char **argv)
>                         printf("Value not found for \"%s\"\n", argv[2]);

This is certainly a bug fix since it was accessing argv[2] without checking that that element was even present, but the more significant outcome of this change is that it now correctly diagnoses when these two commands are called with the wrong number of arguments (just like all the other commands -- except "iterate" -- diagnose incorrect number of arguments). It might make sense, therefore, for the commit message to focus on that improvement and mention the out-of-bounds array access fix as a side-effect. However, that itself is not worth a re-roll.