Re: [PATCH v3 4/6] log: refactor "rev.pending" code in cmd_show()

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Refactor the juggling of "rev.pending" and our replacement for it
> amended in the preceding commit so that:
> ...
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Helped-by: Jeff King <peff@peff.net>

List trailer lines chronologically, please.

This does not look wrong per-se, but given that we might want to
rethink the way the original rev_info is reused inside the loop,
as I mentioned in my review of the previous step, this change may be
irrelevant in the longer term.

I may have said this earlier, but once we start using the "prepare a
blank one, copy it to clear another" pattern, we should stop using
memcpy() and use structure assignment, especially if we are trying to
make our intent clear.

> @@ -745,12 +743,19 @@ int cmd_show(int argc, const char **argv, const char *prefix)
>  		case OBJ_COMMIT:
>  		{
>  			struct object_array old;
> +			struct object_array blank = OBJECT_ARRAY_INIT;
>  
>  			memcpy(&old, &rev.pending, sizeof(old));
> -			rev.pending.nr = rev.pending.alloc = 0;
> -			rev.pending.objects = NULL;
> +			memcpy(&rev.pending, &blank, sizeof(rev.pending));
> +
>  			add_object_array(o, name, &rev.pending);
>  			ret = cmd_log_walk_no_free(&rev);
> +
> +			/*
> +			 * No need for
> +			 * object_array_clear(&pending). It was
> +			 * cleared already in prepare_revision_walk()
> +			 */
>  			memcpy(&rev.pending, &old, sizeof(rev.pending));
>  			break;
>  		}