git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Christian Couder" <christian.couder@gmail.com>,
	"Arjun Sreedharan" <arjun024@gmail.com>,
	git <git@vger.kernel.org>,
	"Christian Couder" <chriscool@tuxfamily.org>,
	"Nguyễn Thái Ngọc" <pclouds@gmail.com>
Subject: [PATCH 1/3] log-tree: make add_name_decoration a public function
Date: Tue, 26 Aug 2014 06:23:36 -0400	[thread overview]
Message-ID: <20140826102335.GA25687@peff.net> (raw)
In-Reply-To: <20140826102051.GA4885@peff.net>

The log-tree code keeps a "struct decoration" hash to show
text decorations for each commit during log traversals. It
makes this available to other files by providing global
access to the hash. This can result in other code adding
entries that do not conform to what log-tree expects.

For example, the bisect code adds its own "dist"
decorations to be shown. Originally the bisect code was
correct, but when the name_decoration code grew a new field
in eb3005e (commit.h: add 'type' to struct name_decoration,
2010-06-19), the bisect code was not updated. As a result,
the log-tree code can access uninitialized memory and even
segfault.

We can fix this by making name_decoration's adding function
public. If all callers use it, then any changes to structi
initialization only need to happen in one place (and because
the members come in as parameters, the compiler can notice a
caller who does not supply enough information).

As a bonus, this also means that the decoration hashes
created by the bisect code will use less memory (previously
we over-allocated space for the distance integer, but not we
format it into a temporary buffer and copy it to the final
flex-array).

Signed-off-by: Jeff King <peff@peff.net>
---
 bisect.c   |  7 ++++---
 commit.h   | 12 ++++++++++++
 log-tree.c | 12 +-----------
 3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/bisect.c b/bisect.c
index d6e851d..df09cbc 100644
--- a/bisect.c
+++ b/bisect.c
@@ -215,11 +215,12 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n
 	}
 	qsort(array, cnt, sizeof(*array), compare_commit_dist);
 	for (p = list, i = 0; i < cnt; i++) {
-		struct name_decoration *r = xmalloc(sizeof(*r) + 100);
+		char buf[100]; /* enough for dist=%d */
 		struct object *obj = &(array[i].commit->object);
 
-		sprintf(r->name, "dist=%d", array[i].distance);
-		r->next = add_decoration(&name_decoration, obj, r);
+		snprintf(buf, sizeof(buf), "dist=%d", array[i].distance);
+		add_name_decoration(DECORATION_NONE, buf, obj);
+
 		p->item = array[i].commit;
 		p = p->next;
 	}
diff --git a/commit.h b/commit.h
index a8cbf52..4902f97 100644
--- a/commit.h
+++ b/commit.h
@@ -33,6 +33,18 @@ struct name_decoration {
 	char name[1];
 };
 
+enum decoration_type {
+	DECORATION_NONE = 0,
+	DECORATION_REF_LOCAL,
+	DECORATION_REF_REMOTE,
+	DECORATION_REF_TAG,
+	DECORATION_REF_STASH,
+	DECORATION_REF_HEAD,
+	DECORATION_GRAFTED,
+};
+
+void add_name_decoration(enum decoration_type type, const char *name, struct object *obj);
+
 struct commit *lookup_commit(const unsigned char *sha1);
 struct commit *lookup_commit_reference(const unsigned char *sha1);
 struct commit *lookup_commit_reference_gently(const unsigned char *sha1,
diff --git a/log-tree.c b/log-tree.c
index 0c53dc1..a821258 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -14,16 +14,6 @@
 
 struct decoration name_decoration = { "object names" };
 
-enum decoration_type {
-	DECORATION_NONE = 0,
-	DECORATION_REF_LOCAL,
-	DECORATION_REF_REMOTE,
-	DECORATION_REF_TAG,
-	DECORATION_REF_STASH,
-	DECORATION_REF_HEAD,
-	DECORATION_GRAFTED,
-};
-
 static char decoration_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_RESET,
 	GIT_COLOR_BOLD_GREEN,	/* REF_LOCAL */
@@ -84,7 +74,7 @@ int parse_decorate_color_config(const char *var, const int ofs, const char *valu
 #define decorate_get_color_opt(o, ix) \
 	decorate_get_color((o)->use_color, ix)
 
-static void add_name_decoration(enum decoration_type type, const char *name, struct object *obj)
+void add_name_decoration(enum decoration_type type, const char *name, struct object *obj)
 {
 	int nlen = strlen(name);
 	struct name_decoration *res = xmalloc(sizeof(struct name_decoration) + nlen);
-- 
2.1.0.346.ga0367b9

  reply	other threads:[~2014-08-26 10:23 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-24 14:17 [PATCH] bisect: save heap memory. allocate only the required amount Arjun Sreedharan
2014-08-24 15:10 ` Stefan Beller
2014-08-24 23:39   ` Junio C Hamano
2014-08-25 13:07     ` Jeff King
2014-08-25 21:36       ` Junio C Hamano
2014-08-26 11:03         ` Jeff King
2014-08-26 11:57           ` Ramsay Jones
2014-08-26 12:14             ` Jeff King
2014-08-26 12:37               ` Ramsay Jones
2014-08-26 12:43                 ` Jeff King
2014-08-26 12:59                   ` Ramsay Jones
2014-08-24 15:32 ` Ramsay Jones
2014-08-24 21:55   ` Arjun Sreedharan
2014-08-25 13:35 ` Jeff King
2014-08-25 14:06   ` Christian Couder
2014-08-25 15:00     ` Jeff King
2014-08-25 18:26       ` Junio C Hamano
2014-08-25 19:35         ` Jeff King
2014-08-25 20:27           ` Arjun Sreedharan
2014-08-25 21:11           ` Junio C Hamano
2014-08-26 10:20             ` [PATCH 0/3] name_decoration cleanups Jeff King
2014-08-26 10:23               ` Jeff King [this message]
2014-08-26 11:48                 ` [PATCH 1/3] log-tree: make add_name_decoration a public function Ramsay Jones
2014-08-26 12:17                   ` Jeff King
2014-08-26 10:23               ` [PATCH 2/3] log-tree: make name_decoration hash static Jeff King
2014-08-26 17:40                 ` Junio C Hamano
2014-08-26 17:43                   ` Jeff King
2014-08-26 19:25                     ` Junio C Hamano
2014-08-26 10:24               ` [PATCH 3/3] log-tree: use FLEX_ARRAY in name_decoration Jeff King
2014-08-27  0:30                 ` Eric Sunshine
2014-08-25 21:14 ` [PATCH] bisect: save heap memory. allocate only the required amount Junio C Hamano
2014-08-26  7:30   ` Arjun Sreedharan

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=20140826102335.GA25687@peff.net \
    --to=peff@peff.net \
    --cc=arjun024@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).