threads / patch / 28146

patch, 3 partsrevision.c: update show_object_with_name() without using malloc()

Subject: [PATCH 3/3] revision.c: update show_object_with_name() without using malloc()

## tl;dr

4 messages between Aug 17, 2011 and Aug 17, 2011. Diffs are folded; open one to read it.

replies: 3people: 1as markdown or json

Junio C Hamano· Aug 17, 2011, 21:30 UTC · lore

[PATCH 0/3] Minor "list-objects" updates

Unify two codepaths that list objects in "rev-list --objects" format, and get rid of unnecessary malloc()/free() done per each line of output.

Junio C Hamano (3):
  rev-list: fix finish_object() call
  revision.c: add show_object_with_name() helper function
  revision.c: update show_object_with_name() without using malloc()
 builtin/rev-list.c |   17 ++---------------
 revision.c         |   41 +++++++++++++++++++++++++++++++++++++++++
 revision.h         |    2 ++
 upload-pack.c      |   15 +--------------
 4 files changed, 46 insertions(+), 29 deletions(-)
-- 
1.7.6.472.g4bfe7c
Junio C Hamano· Aug 17, 2011, 21:30 UTC · re: Junio C Hamano · lore

[PATCH 1/3] rev-list: fix finish_object() call

The callback to traverse_commit_list() are to take linked name_path and a string for the last path component.

If the callee used its parameters, it would have seen duplicated leading paths. In this particular case, the callee does not use this argument but that is not a reason to leave the call broken.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/rev-list.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to builtin/rev-list.c +1 −1
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 56727e8..d789279 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -182,7 +182,7 @@ static void show_object(struct object *obj, const struct name_path *path, const
 	 */
 	const char *ep = strchr(name, '\n');
 
-	finish_object(obj, path, name);
+	finish_object(obj, path, component);
 	if (ep) {
 		printf("%s %.*s\n", sha1_to_hex(obj->sha1),
 		       (int) (ep - name),

-- 
1.7.6.472.g4bfe7c
Junio C Hamano· Aug 17, 2011, 21:30 UTC · re: Junio C Hamano · lore

[PATCH 2/3] revision.c: add show_object_with_name() helper function

There are two copies of traverse_commit_list callback that show the object name followed by pathname the object was found, to produce output similar to "rev-list --objects".

Unify them.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/rev-list.c |   15 +--------------
 revision.c         |   19 +++++++++++++++++++
 revision.h         |    2 ++
 upload-pack.c      |   15 +--------------
 4 files changed, 23 insertions(+), 28 deletions(-)
Show changes to 4 files +23 −28

builtin/rev-list.c, revision.c, revision.h, upload-pack.c

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index d789279..f5ce487 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -176,21 +176,8 @@ static void finish_object(struct object *obj, const struct name_path *path, cons
 
 static void show_object(struct object *obj, const struct name_path *path, const char *component)
 {
-	char *name = path_name(path, component);
-	/* An object with name "foo\n0000000..." can be used to
-	 * confuse downstream "git pack-objects" very badly.
-	 */
-	const char *ep = strchr(name, '\n');
-
 	finish_object(obj, path, component);
-	if (ep) {
-		printf("%s %.*s\n", sha1_to_hex(obj->sha1),
-		       (int) (ep - name),
-		       name);
-	}
-	else
-		printf("%s %s\n", sha1_to_hex(obj->sha1), name);
-	free(name);
+	show_object_with_name(stdout, obj, path, component);
 }
 
 static void show_edge(struct commit *commit)
diff --git a/revision.c b/revision.c
index c46cfaa..c5b38cc 100644
--- a/revision.c
+++ b/revision.c
@@ -40,6 +40,25 @@ char *path_name(const struct name_path *path, const char *name)
 	return n;
 }
 
+void show_object_with_name(FILE *out, struct object *obj, const struct name_path *path, const char *component)
+{
+	char *name = path_name(path, component);
+	const char *ep = strchr(name, '\n');
+
+	/*
+	 * An object with name "foo\n0000000..." can be used to
+	 * confuse downstream "git pack-objects" very badly.
+	 */
+	if (ep) {
+		fprintf(out, "%s %.*s\n", sha1_to_hex(obj->sha1),
+			(int) (ep - name),
+			name);
+	}
+	else
+		fprintf(out, "%s %s\n", sha1_to_hex(obj->sha1), name);
+	free(name);
+}
+
 void add_object(struct object *obj,
 		struct object_array *p,
 		struct name_path *path,
diff --git a/revision.h b/revision.h
index 3d64ada..da00a58 100644
--- a/revision.h
+++ b/revision.h
@@ -185,6 +185,8 @@ struct name_path {
 
 char *path_name(const struct name_path *path, const char *name);
 
+extern void show_object_with_name(FILE *, struct object *, const struct name_path *, const char *);
+
 extern void add_object(struct object *obj,
 		       struct object_array *p,
 		       struct name_path *path,
diff --git a/upload-pack.c b/upload-pack.c
index ce5cbbe..970a1eb 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -85,20 +85,7 @@ static void show_commit(struct commit *commit, void *data)
 
 static void show_object(struct object *obj, const struct name_path *path, const char *component)
 {
-	/* An object with name "foo\n0000000..." can be used to
-	 * confuse downstream git-pack-objects very badly.
-	 */
-	const char *name = path_name(path, component);
-	const char *ep = strchr(name, '\n');
-	if (ep) {
-		fprintf(pack_pipe, "%s %.*s\n", sha1_to_hex(obj->sha1),
-		       (int) (ep - name),
-		       name);
-	}
-	else
-		fprintf(pack_pipe, "%s %s\n",
-				sha1_to_hex(obj->sha1), name);
-	free((char *)name);
+	show_object_with_name(pack_pipe, obj, path, component);
 }
 
 static void show_edge(struct commit *commit)

-- 
1.7.6.472.g4bfe7c
Junio C Hamano· Aug 17, 2011, 21:30 UTC · re: Junio C Hamano · lore

Allocating and then immediately freeing temporary memory a million times when listing a million objects is distasteful.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * It may also be an option to keep a single static strbuf in
   show_object_with_name() and reuse it.
 revision.c |   50 ++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 36 insertions(+), 14 deletions(-)
Show changes to revision.c +36 −14
diff --git a/revision.c b/revision.c
index c5b38cc..072ddac 100644
--- a/revision.c
+++ b/revision.c
@@ -40,23 +40,45 @@ char *path_name(const struct name_path *path, const char *name)
 	return n;
 }
 
+static int show_path_component_truncated(FILE *out, const char *name, int len)
+{
+	int cnt;
+	for (cnt = 0; cnt < len; cnt++) {
+		int ch = name[cnt];
+		if (!ch || ch == '\n')
+			return -1;
+		fputc(ch, out);
+	}
+	return len;
+}
+
+static int show_path_truncated(FILE *out, const struct name_path *path)
+{
+	int emitted, ours;
+
+	if (!path)
+		return 0;
+	emitted = show_path_truncated(out, path->up);
+	if (emitted < 0)
+		return emitted;
+	if (emitted)
+		fputc('/', out);
+	ours = show_path_component_truncated(out, path->elem, path->elem_len);
+	if (ours < 0)
+		return ours;
+	return ours || emitted;
+}
+
 void show_object_with_name(FILE *out, struct object *obj, const struct name_path *path, const char *component)
 {
-	char *name = path_name(path, component);
-	const char *ep = strchr(name, '\n');
+	struct name_path leaf;
+	leaf.up = (struct name_path *)path;
+	leaf.elem = component;
+	leaf.elem_len = strlen(component);
 
-	/*
-	 * An object with name "foo\n0000000..." can be used to
-	 * confuse downstream "git pack-objects" very badly.
-	 */
-	if (ep) {
-		fprintf(out, "%s %.*s\n", sha1_to_hex(obj->sha1),
-			(int) (ep - name),
-			name);
-	}
-	else
-		fprintf(out, "%s %s\n", sha1_to_hex(obj->sha1), name);
-	free(name);
+	fprintf(out, "%s ", sha1_to_hex(obj->sha1));
+	show_path_truncated(out, &leaf);
+	fputc('\n', out);
 }
 
 void add_object(struct object *obj,

-- 
1.7.6.472.g4bfe7c

← back to recent threads