[PATCH v2 3/7] wt-status: introduce wt_status_state_free_buffers()
- From
- Martin Ågren <martin.agren@gmail.com>
- Date
- Sep 27, 2020, 13:15 UTC
- Message-ID
- <c08fed5a01dabcd02a0f0f5dfaf0f87ce3824ca1.1600281351.git.martin.agren@gmail.com>
- In-Reply-To
- <cover.1600281350.git.martin.agren@gmail.com>
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> --- wt-status.h | 7 +++++++ ref-filter.c | 4 +--- worktree.c | 5 ++--- wt-status.c | 11 ++++++++--- 4 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/wt-status.h b/wt-status.h index f1fa0ec1a7..35b44c388e 100644 --- a/wt-status.h +++ b/wt-status.h @@ -151,7 +151,14 @@ void wt_status_add_cut_line(FILE *fp); void wt_status_prepare(struct repository *r, struct wt_status *s); void wt_status_print(struct wt_status *s); void wt_status_collect(struct wt_status *s); +/* + * Frees the buffers allocated by wt_status_collect. + */ void wt_status_collect_free_buffers(struct wt_status *s); +/* + * Frees the buffers of the wt_status_state. + */ +void wt_status_state_free_buffers(struct wt_status_state *s); void wt_status_get_state(struct repository *repo, struct wt_status_state *state, int get_detached_from); diff --git a/ref-filter.c b/ref-filter.c index 8447cb09be..3f63bb01de 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1502,9 +1502,7 @@ char *get_head_description(void) strbuf_addstr(&desc, _("no branch")); strbuf_addch(&desc, ')'); - free(state.branch); - free(state.onto); - free(state.detached_from); + wt_status_state_free_buffers(&state); return strbuf_detach(&desc, NULL); } diff --git a/worktree.c b/worktree.c index cba2e54598..23dd547e44 100644 --- a/worktree.c +++ b/worktree.c @@ -369,8 +369,7 @@ int is_worktree_being_rebased(const struct worktree *wt, state.branch && starts_with(target, "refs/heads/") && !strcmp(state.branch, target + strlen("refs/heads/"))); - free(state.branch); - free(state.onto); + wt_status_state_free_buffers(&state); return found_rebase; } @@ -385,7 +384,7 @@ int is_worktree_being_bisected(const struct worktree *wt, state.branch && starts_with(target, "refs/heads/") && !strcmp(state.branch, target + strlen("refs/heads/")); - free(state.branch); + wt_status_state_free_buffers(&state); return found_rebase; } diff --git a/wt-status.c b/wt-status.c index 3e0b5d8017..5c4cc4805f 100644 --- a/wt-status.c +++ b/wt-status.c @@ -782,9 +782,14 @@ void wt_status_collect(struct wt_status *s) void wt_status_collect_free_buffers(struct wt_status *s) { - free(s->state.branch); - free(s->state.onto); - free(s->state.detached_from); + wt_status_state_free_buffers(&s->state); +} + +void wt_status_state_free_buffers(struct wt_status_state *state) +{ + FREE_AND_NULL(state->branch); + FREE_AND_NULL(state->onto); + FREE_AND_NULL(state->detached_from); } static void wt_longstatus_print_unmerged(struct wt_status *s)
-- 2.28.0.277.g9b3c35fffd