Skip to content

Commit

Permalink
bisect: fix leaking good/bad terms when reading multipe times
Browse files Browse the repository at this point in the history
Even though `read_bisect_terms()` is declared as assigning string
constants, it in fact assigns allocated strings to the `read_bad` and
`read_good` out parameters. The only callers of this function assign the
result to global variables and thus don't have to free them in order to
be leak-free. But that changes when executing the function multiple
times because we'd then overwrite the previous value and thus make it
unreachable.

Fix the function signature and free the previous values. This leak is
exposed by t0630, but plugging it does not make the whole test suite
pass.

Signed-off-by: Patrick Steinhardt <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
  • Loading branch information
pks-t authored and gitster committed Nov 20, 2024
1 parent 65a1b7e commit 79366ad
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 8 deletions.
14 changes: 9 additions & 5 deletions bisect.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ static struct oid_array skipped_revs;

static struct object_id *current_bad_oid;

static const char *term_bad;
static const char *term_good;
static char *term_bad;
static char *term_good;

/* Remember to update object flag allocation in object.h */
#define COUNTED (1u<<16)
Expand Down Expand Up @@ -985,24 +985,28 @@ static void show_commit(struct commit *commit)
* We read them and store them to adapt the messages accordingly.
* Default is bad/good.
*/
void read_bisect_terms(const char **read_bad, const char **read_good)
void read_bisect_terms(char **read_bad, char **read_good)
{
struct strbuf str = STRBUF_INIT;
const char *filename = git_path_bisect_terms();
FILE *fp = fopen(filename, "r");

if (!fp) {
if (errno == ENOENT) {
*read_bad = "bad";
*read_good = "good";
free(*read_bad);
*read_bad = xstrdup("bad");
free(*read_good);
*read_good = xstrdup("good");
return;
} else {
die_errno(_("could not read file '%s'"), filename);
}
} else {
strbuf_getline_lf(&str, fp);
free(*read_bad);
*read_bad = strbuf_detach(&str, NULL);
strbuf_getline_lf(&str, fp);
free(*read_good);
*read_good = strbuf_detach(&str, NULL);
}
strbuf_release(&str);
Expand Down
2 changes: 1 addition & 1 deletion bisect.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix);

int estimate_bisect_steps(int all);

void read_bisect_terms(const char **bad, const char **good);
void read_bisect_terms(char **bad, char **good);

int bisect_clean_state(void);

Expand Down
4 changes: 2 additions & 2 deletions revision.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@

volatile show_early_output_fn_t show_early_output;

static const char *term_bad;
static const char *term_good;
static char *term_bad;
static char *term_good;

implement_shared_commit_slab(revision_sources, char *);

Expand Down

0 comments on commit 79366ad

Please sign in to comment.