Re: bug found on the new git maintenance builtin command
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Sep 14, 2020, 20:48 UTC
- Message-ID
- <0b35829f-a83b-a093-2dc5-0e7d3b42fd15@gmail.com>
- In-Reply-To
- <20200914194938.GA4235@contrib-buster.localdomain>
On 9/14/2020 3:49 PM, Rafael Silva wrote:
> Hi Everyone, > > I found a minor bug when testing the new maintenance built-in command that was > introduced on 679768e2a1 (maintenance: create basic maintenance runner, 2020-08-25) submitted in [1]
Thank you for identifying the original patch! My gut reaction was that this is just in the Part III code which adds subcommands, but that is incorrect.
Show 17 quoted lines
> (gdb) list
> 1628 int cmd_maintenance(int argc, const char **argv, const char *prefix)
> 1629 {
> 1630 if (argc == 2 && !strcmp(argv[1], "-h"))
> 1631 usage(builtin_maintenance_usage);
> 1632
> 1633 fprintf(stdout, "run");
> 1634 if (!strcmp(argv[1], "run"))
> 1635 return maintenance_run(argc - 1, argv + 1, prefix);
> 1636 if (!strcmp(argv[1], "start"))
> 1637 return maintenance_start();
> (gdb) print argc
> $5 = 1
> (gdb) print argv[1]
> $6 = 0x0
>
> Hope all this information helps with the fixing itThank you so much for the report!
The patch below applies to ds/maintenance-part-1, to fix the problem. Hopefully it also merges cleanly with the changes in ds/maintenance-part-3, but I can deal with that when I submit my next re-roll.
Thanks! -Stolee
-- >8 --
From 8cd793e16cd8521f4f8d7ccf2b93492ba444e8e7 Mon Sep 17 00:00:00 2001 From: Derrick Stolee <dstolee@microsoft.com> Date: Mon, 14 Sep 2020 16:42:36 -0400 Subject: [PATCH] maintenance: correctly handle missing subcommand
The maintenance builtin created in 679768e2a12 (maintenance: create basic maintenance runner, 2020-08-25) has a flaw in that it does not protect against a user running "git maintenance" without any additional parameters. Correct this by adding a check on argc before looking for the -h option.
Reported-by: Rafael Silva <rafaeloliveira.cs@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> --- builtin/gc.c | 3 ++- t/t7900-maintenance.sh | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c index c3bcdc1167a..090959350e0 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1027,7 +1027,8 @@ static const char builtin_maintenance_usage[] = N_("git maintenance run [<option int cmd_maintenance(int argc, const char **argv, const char *prefix) { - if (argc == 2 && !strcmp(argv[1], "-h")) + if (argc < 2 || + (argc == 2 && !strcmp(argv[1], "-h"))) usage(builtin_maintenance_usage); if (!strcmp(argv[1], "run")) diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 4f6a04ddb1e..53c883531e4 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -10,7 +10,9 @@ test_expect_success 'help text' ' test_expect_code 129 git maintenance -h 2>err && test_i18ngrep "usage: git maintenance run" err && test_expect_code 128 git maintenance barf 2>err && - test_i18ngrep "invalid subcommand: barf" err + test_i18ngrep "invalid subcommand: barf" err && + test_expect_code 129 git maintenance 2>err && + test_i18ngrep "usage: git maintenance" err ' test_expect_success 'run [--auto|--quiet]' '
-- 2.28.0.vfs.0.0