@matheusbsbs / matheus-bagliano / commits / 962dd7ebc3

Fork 0

wt-status: introduce wt_status_state_free_buffers()

When we have a `struct wt_status_state`, we manually free its `branch`, `onto` and `detached_from`, or sometimes just one or two of them. Provide a function `wt_status_state_free_buffers()` which does the freeing. The callers are still aware of these fields, e.g., they check whether `branch` was populated or not. But this way, they don't need to know about *all* of them, and if `struct wt_status_state` gets more fields, they will not need to learn to free them. Users of `struct wt_status` (which contains a `wt_status_state`) already have `wt_status_collect_free_buffers()` (corresponding to `wt_status_collect()`) which we can also teach to use this new helper. Finally, note that we're currently leaving dangling pointers behind. Some callers work on a stack-allocated struct, where this is obviously ok. But for the users of `run_status()` in builtin/commit.c, there are ample opportunities for someone to mistakenly use those dangling pointers. We seem to be ok for now, but it's a use-after-free waiting to happen. Let's leave NULL-pointers behind instead. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Martin Ågren committed Sep 27, 2020 at 15:15 UTC 962dd7ebc3e76afc2c896d377c319f8140966303

4 files changed +18 -9

ref-filter.c

+1 -3

@@ -1509,9 +1509,7 @@ char *get_head_description(void)

1509 strbuf_addstr(&desc, _("no branch"));

1510 strbuf_addch(&desc, ')');

1511

1512 - free(state.branch);

1513 - free(state.onto);

1514 - free(state.detached_from);

1512 + wt_status_state_free_buffers(&state);

1513 return strbuf_detach(&desc, NULL);

1514 }

1515

worktree.c

+2 -3

@@ -357,8 +357,7 @@ int is_worktree_being_rebased(const struct worktree *wt,

357 state.branch &&

358 starts_with(target, "refs/heads/") &&

359 !strcmp(state.branch, target + strlen("refs/heads/")));

360 - free(state.branch);

361 - free(state.onto);

360 + wt_status_state_free_buffers(&state);

361 return found_rebase;

362 }

363

@@ -373,7 +372,7 @@ int is_worktree_being_bisected(const struct worktree *wt,

372 state.branch &&

373 starts_with(target, "refs/heads/") &&

374 !strcmp(state.branch, target + strlen("refs/heads/"));

376 - free(state.branch);

375 + wt_status_state_free_buffers(&state);

376 return found_rebase;

377 }

378

wt-status.c

+8 -3

@@ -778,9 +778,14 @@ void wt_status_collect(struct wt_status *s)

778

779 void wt_status_collect_free_buffers(struct wt_status *s)

780 {

781 - free(s->state.branch);

782 - free(s->state.onto);

783 - free(s->state.detached_from);

781 + wt_status_state_free_buffers(&s->state);

782 +}

783 +

784 +void wt_status_state_free_buffers(struct wt_status_state *state)

785 +{

786 + FREE_AND_NULL(state->branch);

787 + FREE_AND_NULL(state->onto);

788 + FREE_AND_NULL(state->detached_from);

789 }

790

791 static void wt_longstatus_print_unmerged(struct wt_status *s)

wt-status.h

@@ -151,7 +151,14 @@ void wt_status_add_cut_line(FILE *fp);

151 void wt_status_prepare(struct repository *r, struct wt_status *s);

152 void wt_status_print(struct wt_status *s);

153 void wt_status_collect(struct wt_status *s);

154 +/*

155 + * Frees the buffers allocated by wt_status_collect.

156 + */

157 void wt_status_collect_free_buffers(struct wt_status *s);

158 +/*

159 + * Frees the buffers of the wt_status_state.

160 + */

161 +void wt_status_state_free_buffers(struct wt_status_state *s);

162 void wt_status_get_state(struct repository *repo,

163 struct wt_status_state *state,

164 int get_detached_from);