biultin/rev-parse: fix memory leaks in `--parseopt` mode
We have a bunch of memory leaks in git-rev-parse(1)'s `--parseopt` mode. Refactor the code to use `struct strvec`s to make it easier for us to track the lifecycle of those leaking variables and then free them. While at it, remove the unneeded static lifetime for some of the variables. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Jun 11, 2024 at 11:19 UTC
afb0653d2373dc72ad4a9c89b7d17b660d194f04
3 files changed
+32
-23
builtin/rev-parse.c
+30
-23
@@ -423,12 +423,12 @@ static char *findspace(const char *s)
423
424
static int cmd_parseopt(int argc, const char **argv, const char *prefix)
425
{
426
- static int keep_dashdash = 0, stop_at_non_option = 0;
427
- static char const * const parseopt_usage[] = {
426
+ int keep_dashdash = 0, stop_at_non_option = 0;
427
+ char const * const parseopt_usage[] = {
428
N_("git rev-parse --parseopt [<options>] -- [<args>...]"),
429
NULL
430
};
431
- static struct option parseopt_opts[] = {
431
+ struct option parseopt_opts[] = {
432
OPT_BOOL(0, "keep-dashdash", &keep_dashdash,
433
N_("keep the `--` passed as an arg")),
434
OPT_BOOL(0, "stop-at-non-option", &stop_at_non_option,
@@ -438,12 +438,11 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
438
N_("output in stuck long form")),
439
OPT_END(),
440
};
441
- static const char * const flag_chars = "*=?!";
442
-
441
struct strbuf sb = STRBUF_INIT, parsed = STRBUF_INIT;
444
- const char **usage = NULL;
442
+ struct strvec longnames = STRVEC_INIT;
443
+ struct strvec usage = STRVEC_INIT;
444
struct option *opts = NULL;
446
- int onb = 0, osz = 0, unb = 0, usz = 0;
445
+ size_t opts_nr = 0, opts_alloc = 0;
446
447
strbuf_addstr(&parsed, "set --");
448
argc = parse_options(argc, argv, prefix, parseopt_opts, parseopt_usage,
@@ -453,16 +452,16 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
452
453
/* get the usage up to the first line with a -- on it */
454
for (;;) {
455
+ strbuf_reset(&sb);
456
if (strbuf_getline(&sb, stdin) == EOF)
457
die(_("premature end of input"));
458
- ALLOC_GROW(usage, unb + 1, usz);
458
if (!strcmp("--", sb.buf)) {
460
- if (unb < 1)
459
+ if (!usage.nr)
460
die(_("no usage string given before the `--' separator"));
462
- usage[unb] = NULL;
461
break;
462
}
465
- usage[unb++] = strbuf_detach(&sb, NULL);
463
+
464
+ strvec_push(&usage, sb.buf);
465
}
466
467
/* parse: (<short>|<short>,<long>|<long>)[*=?!]*<arghint>? SP+ <help> */
@@ -474,10 +473,10 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
473
if (!sb.len)
474
continue;
475
477
- ALLOC_GROW(opts, onb + 1, osz);
478
- memset(opts + onb, 0, sizeof(opts[onb]));
476
+ ALLOC_GROW(opts, opts_nr + 1, opts_alloc);
477
+ memset(opts + opts_nr, 0, sizeof(*opts));
478
480
- o = &opts[onb++];
479
+ o = &opts[opts_nr++];
480
help = findspace(sb.buf);
481
if (!help || sb.buf == help) {
482
o->type = OPTION_GROUP;
@@ -494,20 +493,22 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
493
o->callback = &parseopt_dump;
494
495
/* name(s) */
497
- s = strpbrk(sb.buf, flag_chars);
496
+ s = strpbrk(sb.buf, "*=?!");
497
if (!s)
498
s = help;
499
500
if (s == sb.buf)
501
die(_("missing opt-spec before option flags"));
502
504
- if (s - sb.buf == 1) /* short option only */
503
+ if (s - sb.buf == 1) { /* short option only */
504
o->short_name = *sb.buf;
506
- else if (sb.buf[1] != ',') /* long option only */
507
- o->long_name = xmemdupz(sb.buf, s - sb.buf);
508
- else {
505
+ } else if (sb.buf[1] != ',') { /* long option only */
506
+ o->long_name = strvec_pushf(&longnames, "%.*s",
507
+ (int)(s - sb.buf), sb.buf);
508
+ } else {
509
o->short_name = *sb.buf;
510
- o->long_name = xmemdupz(sb.buf + 2, s - sb.buf - 2);
510
+ o->long_name = strvec_pushf(&longnames, "%.*s",
511
+ (int)(s - sb.buf - 2), sb.buf + 2);
512
}
513
514
/* flags */
@@ -537,9 +538,9 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
538
strbuf_release(&sb);
539
540
/* put an OPT_END() */
540
- ALLOC_GROW(opts, onb + 1, osz);
541
- memset(opts + onb, 0, sizeof(opts[onb]));
542
- argc = parse_options(argc, argv, prefix, opts, usage,
541
+ ALLOC_GROW(opts, opts_nr + 1, opts_alloc);
542
+ memset(opts + opts_nr, 0, sizeof(*opts));
543
+ argc = parse_options(argc, argv, prefix, opts, usage.v,
544
(keep_dashdash ? PARSE_OPT_KEEP_DASHDASH : 0) |
545
(stop_at_non_option ? PARSE_OPT_STOP_AT_NON_OPTION : 0) |
546
PARSE_OPT_SHELL_EVAL);
@@ -547,7 +548,13 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
548
strbuf_addstr(&parsed, " --");
549
sq_quote_argv(&parsed, argv);
550
puts(parsed.buf);
551
+
552
strbuf_release(&parsed);
553
+ strbuf_release(&sb);
554
+ strvec_clear(&longnames);
555
+ strvec_clear(&usage);
556
+ free((char *) opts->help);
557
+ free(opts);
558
return 0;
559
}
560
t/t5150-request-pull.sh
+1
@@ -5,6 +5,7 @@ test_description='Test workflows involving pull request.'
5
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
6
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
7
8
+TEST_PASSES_SANITIZE_LEAK=true
9
. ./test-lib.sh
10
11
if ! test_have_prereq PERL
t/t7006-pager.sh
+1
@@ -2,6 +2,7 @@
2
3
test_description='Test automatic use of a pager.'
4
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
. "$TEST_DIRECTORY"/lib-pager.sh
8
. "$TEST_DIRECTORY"/lib-terminal.sh