git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/3] log-tree: make add_name_decoration a public function

From
Jeff King <peff@peff.net>
Date
Aug 26, 2014, 10:23 UTC
Message-ID
<20140826102335.GA25687@peff.net>
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
Previous: Jeff KingNext: Ramsay Jones
Message 22 of 32 in “bisect: save heap memory. allocate only the required amount”
  1. bisect: save heap memory. allocate only the required amountArjun Sreedharan, Aug 24, 2014
  2. Stefan BellerAug 24, 2014
  3. Junio C HamanoAug 24, 2014
  4. Jeff KingAug 25, 2014
  5. Junio C HamanoAug 25, 2014
  6. Jeff KingAug 26, 2014
  7. Ramsay JonesAug 26, 2014
  8. Jeff KingAug 26, 2014
  9. Ramsay JonesAug 26, 2014
  10. Jeff KingAug 26, 2014
  11. Ramsay JonesAug 26, 2014
  12. Ramsay JonesAug 24, 2014
  13. bisect: save heap memory. allocate only the required amountArjun Sreedharan, Aug 24, 2014
  14. Jeff KingAug 25, 2014
  15. Christian CouderAug 25, 2014
  16. Jeff KingAug 25, 2014
  17. Junio C HamanoAug 25, 2014
  18. Jeff KingAug 25, 2014
  19. Arjun SreedharanAug 25, 2014
  20. Junio C HamanoAug 25, 2014
  21. 0/3 name_decoration cleanupsJeff King, Aug 26, 2014
  22. 1/3 log-tree: make add_name_decoration a public functionJeff King, Aug 26, 2014
  23. Ramsay JonesAug 26, 2014
  24. Jeff KingAug 26, 2014
  25. 2/3 log-tree: make name_decoration hash staticJeff King, Aug 26, 2014
  26. Junio C HamanoAug 26, 2014
  27. Jeff KingAug 26, 2014
  28. Junio C HamanoAug 26, 2014
  29. 3/3 log-tree: use FLEX_ARRAY in name_decorationJeff King, Aug 26, 2014
  30. Eric SunshineAug 27, 2014
  31. Junio C HamanoAug 25, 2014
  32. Arjun SreedharanAug 26, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.