On Fri, Apr 19, 2013 at 06:44:49PM +0200, Michael J Gruber wrote:

> @@ -820,12 +820,13 @@ int cmd_grep(int argc, const char **argv, const char 
> *prefix)
>       for (i = 0; i < argc; i++) {
>               const char *arg = argv[i];
>               unsigned char sha1[20];
> +             struct object_context oc;
>               /* Is it a rev? */
> -             if (!get_sha1(arg, sha1)) {
> +             if (!get_sha1_with_context(arg, 0, sha1, &oc)) {
>                       struct object *object = parse_object_or_die(sha1, arg);
>                       if (!seen_dashdash)
>                               verify_non_filename(prefix, arg);
> -                     add_object_array(object, arg, &list);
> +                     add_object_array_with_context(object, arg, &list, 
> xmemdupz(&oc, sizeof(struct object_context)));

Hrm. I'm not excited about the extra allocation here. Who frees it?

> +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_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);
> +}

And this mass of almost-the-same functions is gross, too, especially
given that the object_context contains a mode itself.

Unfortunately, I'm not sure if I have a more pleasant suggestion. I seem
to recall wrestling with this issue during the last round, too.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to