From 8eee9f9277b6e38ec46c84f4ca3be5d988ca0a33 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 11 Feb 2016 17:24:18 -0500 Subject: show_object_with_name: simplify by using path_name() When "git rev-list" shows an object with its associated path name, it does so by walking the name_path linked list and printing each component (stopping at any embedded NULs or newlines). We'd like to eventually get rid of name_path entirely in favor of a single buffer, and dropping this custom printing code is part of that. As a first step, let's use path_name() to format the list into a single buffer, and print that. This is strictly less efficient than the original, but it's a temporary step in the refactoring; our end game will be to get the fully formatted name in the first place. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- revision.c | 40 ++++++---------------------------------- 1 file changed, 6 insertions(+), 34 deletions(-) (limited to 'revision.c') diff --git a/revision.c b/revision.c index 0b322b4fdc..cf544b6e96 100644 --- a/revision.c +++ b/revision.c @@ -45,46 +45,18 @@ 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) { - struct name_path leaf; - leaf.up = (struct name_path *)path; - leaf.elem = component; - leaf.elem_len = strlen(component); + char *name = path_name(path, component); + char *p; fprintf(out, "%s ", sha1_to_hex(obj->sha1)); - show_path_truncated(out, &leaf); + for (p = name; *p && *p != '\n'; p++) + fputc(*p, out); fputc('\n', out); + + free(name); } static void mark_blob_uninteresting(struct blob *blob) -- cgit v1.2.3 From f3badaed5106a16499d0fae31a382f9047b272d7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 11 Feb 2016 17:26:18 -0500 Subject: list-objects: convert name_path to a strbuf The "struct name_path" data is examined in only two places: we generate it in process_tree(), and we convert it to a single string in path_name(). Everyone else just passes it through to those functions. We can further note that process_tree() already keeps a single strbuf with the leading tree path, for use with tree_entry_interesting(). Instead of building a separate name_path linked list, let's just use the one we already build in "base". This reduces the amount of code (especially tricky code in path_name() which did not check for integer overflows caused by deep or large pathnames). It is also more efficient in some instances. Any time we were using tree_entry_interesting, we were building up the strbuf anyway, so this is an immediate and obvious win there. In cases where we were not, we trade off storing "pathname/" in a strbuf on the heap for each level of the path, instead of two pointers and an int on the stack (with one pointer into the tree object). On a 64-bit system, the latter is 20 bytes; so if path components are less than that on average, this has lower peak memory usage. In practice it probably doesn't matter either way; we are already holding in memory all of the tree objects leading up to each pathname, and for normal-depth pathnames, we are only talking about hundreds of bytes. This patch leaves "struct name_path" as a thin wrapper around the strbuf, to avoid disrupting callbacks. We should fix them, but leaving it out makes this diff easier to view. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- revision.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) (limited to 'revision.c') diff --git a/revision.c b/revision.c index cf544b6e96..f8c30341ed 100644 --- a/revision.c +++ b/revision.c @@ -23,26 +23,11 @@ volatile show_early_output_fn_t show_early_output; char *path_name(const struct name_path *path, const char *name) { - const struct name_path *p; - char *n, *m; - int nlen = strlen(name); - int len = nlen + 1; - - for (p = path; p; p = p->up) { - if (p->elem_len) - len += p->elem_len + 1; - } - n = xmalloc(len); - m = n + len - (nlen + 1); - strcpy(m, name); - for (p = path; p; p = p->up) { - if (p->elem_len) { - m -= p->elem_len + 1; - memcpy(m, p->elem, p->elem_len); - m[p->elem_len] = '/'; - } - } - return n; + struct strbuf ret = STRBUF_INIT; + if (path) + strbuf_addbuf(&ret, path->base); + strbuf_addstr(&ret, name); + return strbuf_detach(&ret, NULL); } void show_object_with_name(FILE *out, struct object *obj, -- cgit v1.2.3 From dc06dc880013d48f2b09c6b4295419382f3b8230 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 11 Feb 2016 17:26:44 -0500 Subject: list-objects: drop name_path entirely In the previous commit, we left name_path as a thin wrapper around a strbuf. This patch drops it entirely. As a result, every show_object_fn callback needs to be adjusted. However, none of their code needs to be changed at all, because the only use was to pass it to path_name(), which now handles the bare strbuf. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- revision.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'revision.c') diff --git a/revision.c b/revision.c index f8c30341ed..1ff6709d4b 100644 --- a/revision.c +++ b/revision.c @@ -21,17 +21,17 @@ volatile show_early_output_fn_t show_early_output; -char *path_name(const struct name_path *path, const char *name) +char *path_name(struct strbuf *path, const char *name) { struct strbuf ret = STRBUF_INIT; if (path) - strbuf_addbuf(&ret, path->base); + strbuf_addbuf(&ret, path); strbuf_addstr(&ret, name); return strbuf_detach(&ret, NULL); } void show_object_with_name(FILE *out, struct object *obj, - const struct name_path *path, const char *component) + struct strbuf *path, const char *component) { char *name = path_name(path, component); char *p; -- cgit v1.2.3 From 2824e1841b99393d2469c495253d547c643bd8f1 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 11 Feb 2016 17:28:36 -0500 Subject: list-objects: pass full pathname to callbacks When we find a blob at "a/b/c", we currently pass this to our show_object_fn callbacks as two components: "a/b/" and "c". Callbacks which want the full value then call path_name(), which concatenates the two. But this is an inefficient interface; the path is a strbuf, and we could simply append "c" to it temporarily, then roll back the length, without creating a new copy. So we could improve this by teaching the callsites of path_name() this trick (and there are only 3). But we can also notice that no callback actually cares about the broken-down representation, and simply pass each callback the full path "a/b/c" as a string. The callback code becomes even simpler, then, as we do not have to worry about freeing an allocated buffer, nor rolling back our modification to the strbuf. This is theoretically less efficient, as some callbacks would not bother to format the final path component. But in practice this is not measurable. Since we use the same strbuf over and over, our work to grow it is amortized, and we really only pay to memcpy a few bytes. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- revision.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) (limited to 'revision.c') diff --git a/revision.c b/revision.c index 1ff6709d4b..871812db2a 100644 --- a/revision.c +++ b/revision.c @@ -21,27 +21,14 @@ volatile show_early_output_fn_t show_early_output; -char *path_name(struct strbuf *path, const char *name) +void show_object_with_name(FILE *out, struct object *obj, const char *name) { - struct strbuf ret = STRBUF_INIT; - if (path) - strbuf_addbuf(&ret, path); - strbuf_addstr(&ret, name); - return strbuf_detach(&ret, NULL); -} - -void show_object_with_name(FILE *out, struct object *obj, - struct strbuf *path, const char *component) -{ - char *name = path_name(path, component); - char *p; + const char *p; fprintf(out, "%s ", sha1_to_hex(obj->sha1)); for (p = name; *p && *p != '\n'; p++) fputc(*p, out); fputc('\n', out); - - free(name); } static void mark_blob_uninteresting(struct blob *blob) -- cgit v1.2.3