Re: [PATCH v3 4/6] log: refactor "rev.pending" code in cmd_show()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 3, 2022, 17:59 UTC
- Message-ID
- <xmqqo7x19pif.fsf@gitster.g>
- In-Reply-To
- <patch-v3-4.6-fd474666e7c-20220802T152925Z-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 5 quoted lines
> 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.
Show 22 quoted lines
> @@ -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;
> }