From 68f492359e29bbdf633201406d0646deee2b298c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 15 Oct 2014 18:34:19 -0400 Subject: object_array: factor out slopbuf-freeing logic This is not a lot of code, but it's a logical construct that should not need to be repeated (and we are about to add a third repetition). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- object.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'object.c') diff --git a/object.c b/object.c index ca9d790f4d..60f4864632 100644 --- a/object.c +++ b/object.c @@ -355,6 +355,16 @@ void add_object_array_with_context(struct object *obj, const char *name, struct add_object_array_with_mode_context(obj, name, array, S_IFINVALID, context); } +/* + * Free all memory associated with an entry; the result is + * in an unspecified state and should not be examined. + */ +static void object_array_release_entry(struct object_array_entry *ent) +{ + if (ent->name != object_array_slopbuf) + free(ent->name); +} + void object_array_filter(struct object_array *array, object_array_each_func_t want, void *cb_data) { @@ -367,8 +377,7 @@ void object_array_filter(struct object_array *array, objects[dst] = objects[src]; dst++; } else { - if (objects[src].name != object_array_slopbuf) - free(objects[src].name); + object_array_release_entry(&objects[src]); } } array->nr = dst; @@ -400,8 +409,7 @@ void object_array_remove_duplicates(struct object_array *array) objects[array->nr] = objects[src]; array->nr++; } else { - if (objects[src].name != object_array_slopbuf) - free(objects[src].name); + object_array_release_entry(&objects[src]); } } } -- cgit v1.2.3 From 46be823124bb6a6ff0e06dc19c327b599ed97c72 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 15 Oct 2014 18:34:34 -0400 Subject: object_array: add a "clear" function There's currently no easy way to free the memory associated with an object_array (and in most cases, we simply leak the memory in a rev_info's pending array). Let's provide a helper to make this easier to handle. We can make use of it in list-objects.c, which does the same thing by hand (but fails to free the "name" field of each entry, potentially leaking memory). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- list-objects.c | 7 +------ object.c | 10 ++++++++++ object.h | 6 ++++++ 3 files changed, 17 insertions(+), 6 deletions(-) (limited to 'object.c') diff --git a/list-objects.c b/list-objects.c index 3595ee7a22..fad6808aad 100644 --- a/list-objects.c +++ b/list-objects.c @@ -228,11 +228,6 @@ void traverse_commit_list(struct rev_info *revs, die("unknown pending object %s (%s)", sha1_to_hex(obj->sha1), name); } - if (revs->pending.nr) { - free(revs->pending.objects); - revs->pending.nr = 0; - revs->pending.alloc = 0; - revs->pending.objects = NULL; - } + object_array_clear(&revs->pending); strbuf_release(&base); } diff --git a/object.c b/object.c index 60f4864632..6aeb1bbbe3 100644 --- a/object.c +++ b/object.c @@ -383,6 +383,16 @@ void object_array_filter(struct object_array *array, array->nr = dst; } +void object_array_clear(struct object_array *array) +{ + int i; + for (i = 0; i < array->nr; i++) + object_array_release_entry(&array->objects[i]); + free(array->objects); + array->objects = NULL; + array->nr = array->alloc = 0; +} + /* * Return true iff array already contains an entry with name. */ diff --git a/object.h b/object.h index e028ced74c..2a755a2373 100644 --- a/object.h +++ b/object.h @@ -133,6 +133,12 @@ void object_array_filter(struct object_array *array, */ void object_array_remove_duplicates(struct object_array *array); +/* + * Remove any objects from the array, freeing all used memory; afterwards + * the array is ready to store more objects with add_object_array(). + */ +void object_array_clear(struct object_array *array); + void clear_object_flags(unsigned flags); #endif /* OBJECT_H */ -- cgit v1.2.3 From 9e0c3c4fcdf3775a9e0256ee231efa4698297a0e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 15 Oct 2014 18:42:57 -0400 Subject: make add_object_array_with_context interface more sane When you resolve a sha1, you can optionally keep any context found during the resolution, including the path and mode of a tree entry (e.g., when looking up "HEAD:subdir/file.c"). The add_object_array_with_context function lets you then attach that context to an entry in a list. Unfortunately, the interface for doing so is horrible. The object_context structure is large and most object_array users do not use it. Therefore we keep a pointer to the structure to avoid burdening other users too much. But that means when we do use it that we must allocate the struct ourselves. And the struct contains a fixed PATH_MAX-sized buffer, which makes this wholly unsuitable for any large arrays. We can observe that there is only a single user of the "with_context" variant: builtin/grep.c. And in that use case, the only element we care about is the path. We can therefore store only the path as a pointer (the context's mode field was redundant with the object_array_entry itself, and nobody actually cared about the surrounding tree). This still requires a strdup of the pathname, but at least we are only consuming the minimum amount of memory for each string. We can also handle the copying ourselves in add_object_array_*, and free it as appropriate in object_array_release_entry. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/grep.c | 8 ++++---- object.c | 23 +++++++++-------------- object.h | 4 ++-- 3 files changed, 15 insertions(+), 20 deletions(-) (limited to 'object.c') diff --git a/builtin/grep.c b/builtin/grep.c index c86a142f30..4063882f06 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -456,10 +456,10 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec, } static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec, - struct object *obj, const char *name, struct object_context *oc) + struct object *obj, const char *name, const char *path) { if (obj->type == OBJ_BLOB) - return grep_sha1(opt, obj->sha1, name, 0, oc ? oc->path : NULL); + return grep_sha1(opt, obj->sha1, name, 0, path); if (obj->type == OBJ_COMMIT || obj->type == OBJ_TREE) { struct tree_desc tree; void *data; @@ -501,7 +501,7 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec, for (i = 0; i < nr; i++) { struct object *real_obj; real_obj = deref_tag(list->objects[i].item, NULL, 0); - if (grep_object(opt, pathspec, real_obj, list->objects[i].name, list->objects[i].context)) { + if (grep_object(opt, pathspec, real_obj, list->objects[i].name, list->objects[i].path)) { hit = 1; if (opt->status_only) break; @@ -821,7 +821,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) struct object *object = parse_object_or_die(sha1, arg); if (!seen_dashdash) verify_non_filename(prefix, arg); - add_object_array_with_context(object, arg, &list, xmemdupz(&oc, sizeof(struct object_context))); + add_object_array_with_path(object, arg, &list, oc.mode, oc.path); continue; } if (!strcmp(arg, "--")) { diff --git a/object.c b/object.c index 6aeb1bbbe3..df86bdd5a5 100644 --- a/object.c +++ b/object.c @@ -307,10 +307,9 @@ int object_list_contains(struct object_list *list, struct object *obj) */ static char object_array_slopbuf[1]; -static void add_object_array_with_mode_context(struct object *obj, const char *name, - struct object_array *array, - unsigned mode, - struct object_context *context) +void add_object_array_with_path(struct object *obj, const char *name, + struct object_array *array, + unsigned mode, const char *path) { unsigned nr = array->nr; unsigned alloc = array->alloc; @@ -333,7 +332,10 @@ static void add_object_array_with_mode_context(struct object *obj, const char *n else entry->name = xstrdup(name); entry->mode = mode; - entry->context = context; + if (path) + entry->path = xstrdup(path); + else + entry->path = NULL; array->nr = ++nr; } @@ -344,15 +346,7 @@ void add_object_array(struct object *obj, const char *name, struct object_array void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode) { - add_object_array_with_mode_context(obj, name, array, mode, NULL); -} - -void add_object_array_with_context(struct object *obj, const char *name, struct object_array *array, struct object_context *context) -{ - if (context) - add_object_array_with_mode_context(obj, name, array, context->mode, context); - else - add_object_array_with_mode_context(obj, name, array, S_IFINVALID, context); + add_object_array_with_path(obj, name, array, mode, NULL); } /* @@ -363,6 +357,7 @@ static void object_array_release_entry(struct object_array_entry *ent) { if (ent->name != object_array_slopbuf) free(ent->name); + free(ent->path); } void object_array_filter(struct object_array *array, diff --git a/object.h b/object.h index 2a755a2373..e5178a516d 100644 --- a/object.h +++ b/object.h @@ -18,8 +18,8 @@ struct object_array { * empty string. */ char *name; + char *path; unsigned mode; - struct object_context *context; } *objects; }; @@ -115,7 +115,7 @@ int object_list_contains(struct object_list *list, struct object *obj); /* Object array handling .. */ void add_object_array(struct object *obj, const char *name, struct object_array *array); void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode); -void add_object_array_with_context(struct object *obj, const char *name, struct object_array *array, struct object_context *context); +void add_object_array_with_path(struct object *obj, const char *name, struct object_array *array, unsigned mode, const char *path); typedef int (*object_array_each_func_t)(struct object_array_entry *, void *); -- cgit v1.2.3 From 189a1222493f73977291f57d0f2030e982aff282 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 18 Oct 2014 22:03:19 -0400 Subject: drop add_object_array_with_mode This is a thin compatibility wrapper around add_pending_object_with_path. But the only caller is add_object_array, which is itself just a thin compatibility wrapper. There are no external callers, so we can just remove this middle wrapper. Noticed-by: Ramsay Jones Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- object.c | 7 +------ object.h | 1 - 2 files changed, 1 insertion(+), 7 deletions(-) (limited to 'object.c') diff --git a/object.c b/object.c index df86bdd5a5..23d6c96719 100644 --- a/object.c +++ b/object.c @@ -341,12 +341,7 @@ void add_object_array_with_path(struct object *obj, const char *name, void add_object_array(struct object *obj, const char *name, struct object_array *array) { - add_object_array_with_mode(obj, name, array, S_IFINVALID); -} - -void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode) -{ - add_object_array_with_path(obj, name, array, mode, NULL); + add_object_array_with_path(obj, name, array, S_IFINVALID, NULL); } /* diff --git a/object.h b/object.h index e5178a516d..6416247def 100644 --- a/object.h +++ b/object.h @@ -114,7 +114,6 @@ int object_list_contains(struct object_list *list, struct object *obj); /* Object array handling .. */ void add_object_array(struct object *obj, const char *name, struct object_array *array); -void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode); void add_object_array_with_path(struct object *obj, const char *name, struct object_array *array, unsigned mode, const char *path); typedef int (*object_array_each_func_t)(struct object_array_entry *, void *); -- cgit v1.2.3