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 <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/grep.c b/builtin/grep.c
index c86a142f308d614daefd459fd9f9ddebe03f4cb8..4063882f06536ad5b3f1292a5b66e534ad4688af 100644 (file)
--- 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 6aeb1bbbe35b79f834c5506f523beeb4e92d8790..df86bdd5a5e944f673fc4bddb6383476c9b0e8ae 100644 (file)
--- 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 2a755a23734ca8dc8188ce4507259e49bbfdee7b..e5178a516de7c36dc40bc2f994e2acced8ca37a4 100644 (file)
--- 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 *);