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>

diff --git a/ref-filter.c b/ref-filter.c
index 8ba0e31915021e3d60b814332c6a10c10f3a147e..ea00ca38c8b186316d70043014305783200e686d 100644 (file)
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1509,9 +1509,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 62217b4a6b26b4efad52643d25f21e7c98a2fa3e..62a7eb9342802ce1e103bffcdc19597060191bae 100644 (file)
--- a/worktree.c
+++ b/worktree.c
@@ -357,8 +357,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;
 }
 
@@ -373,7 +372,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 4a7483ee7d17bda0fe4d8442eb20e969104155bc..4345eb1749794f62861137b3d438d2e7e9779919 100644 (file)
--- a/wt-status.c
+++ b/wt-status.c
@@ -778,9 +778,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)

diff --git a/wt-status.h b/wt-status.h
index f1fa0ec1a750a92967c35cafe2fdb25a32873d5a..35b44c388edf0cd75cca00e1458b9995c8001538 100644 (file)
--- 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);