All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Martin Ågren" <martin.agren@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 3/8] wt-status: introduce wt_status_state_free_buffers()
Date: Thu, 10 Sep 2020 21:03:37 +0200	[thread overview]
Message-ID: <c08fed5a01dabcd02a0f0f5dfaf0f87ce3824ca1.1599762679.git.martin.agren@gmail.com> (raw)
In-Reply-To: <cover.1599762679.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


  parent reply	other threads:[~2020-09-10 19:07 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-10 19:03 [PATCH 0/8] various wt-status/worktree cleanups Martin Ågren
2020-09-10 19:03 ` [PATCH 1/8] wt-status: replace sha1 mentions with oid Martin Ågren
2020-09-10 19:03 ` [PATCH 2/8] wt-status: print to s->fp, not stdout Martin Ågren
2020-09-10 19:03 ` Martin Ågren [this message]
2020-09-10 19:03 ` [PATCH 4/8] worktree: drop useless call to strbuf_reset Martin Ågren
2020-09-10 19:15   ` Eric Sunshine
2020-09-10 19:39     ` Martin Ågren
2020-09-10 19:49       ` Eric Sunshine
2020-09-12 14:02         ` Martin Ågren
2020-09-10 19:03 ` [PATCH 5/8] worktree: update renamed variable in comment Martin Ågren
2020-09-10 19:03 ` [PATCH 6/8] worktree: rename copy-pasted variable Martin Ågren
2020-09-10 20:29   ` Junio C Hamano
2020-09-12 14:01     ` Martin Ågren
2020-09-27 13:29       ` Martin Ågren
2020-09-10 19:03 ` [PATCH 7/8] worktree: use skip_prefix to parse target Martin Ågren
2020-09-10 19:03 ` [PATCH 8/8] worktree: simplify search for unique worktree Martin Ågren
2020-09-10 19:28   ` Eric Sunshine
2020-09-10 19:48     ` Martin Ågren
2020-09-10 20:01       ` Eric Sunshine
2020-09-10 21:08         ` Junio C Hamano
2020-09-12  3:49 ` [PATCH 0/8] various wt-status/worktree cleanups Taylor Blau
2020-09-12 14:03   ` Martin Ågren
2020-09-27 13:15 ` [PATCH v2 0/7] " Martin Ågren
2020-09-27 13:15   ` [PATCH v2 1/7] wt-status: replace sha1 mentions with oid Martin Ågren
2020-09-27 13:15   ` [PATCH v2 2/7] wt-status: print to s->fp, not stdout Martin Ågren
2020-09-27 13:15   ` [PATCH v2 3/7] wt-status: introduce wt_status_state_free_buffers() Martin Ågren
2020-09-27 13:15   ` [PATCH v2 4/7] worktree: inline `worktree_ref()` into its only caller Martin Ågren
2020-09-28  5:30     ` Eric Sunshine
2020-09-28  6:57       ` Martin Ågren
2020-09-28  7:16         ` Eric Sunshine
2020-09-27 13:15   ` [PATCH v2 5/7] worktree: update renamed variable in comment Martin Ågren
2020-09-27 13:15   ` [PATCH v2 6/7] worktree: rename copy-pasted variable Martin Ågren
2020-09-27 13:15   ` [PATCH v2 7/7] worktree: use skip_prefix to parse target Martin Ågren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c08fed5a01dabcd02a0f0f5dfaf0f87ce3824ca1.1599762679.git.martin.agren@gmail.com \
    --to=martin.agren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.