Nguyễn Thái Ngọc Duy  <pclo...@gmail.com> writes:

> -     refname = commit->util;
> +     refname = *revision_sources_peek(&revision_sources, commit);

At first glance, I thought that the reason why this uses _peek() is
because the "sources" is expected to only sparsely be populated,
perhaps because get_tags_and_duplicates() annotates only the tips
mentioned on the command line via rev_cmdline mechanism and this
code does not want to auto-vivify the slot, only to read NULL from
it.

But the code that follows this point liberally uses refname without
checking if it is NULL, so I am not quite sure what is going on. In
any case, wouldn't *slab_peek() an anti-pattern?  You use _peek()
because you expect that a slot is not yet allocated for a commit,
you desire not to vivify all the slots for all the commits, and
instead you are prepared to see a NULL returned from the call.  But
I do not think that is what is happening here, so shouldn't it be
using _at() instead of _peek()?

>       if (anonymize) {
>               refname = anonymize_refname(refname);

>               anonymize_ident_line(&committer, &committer_end);
> @@ -862,10 +864,11 @@ static void get_tags_and_duplicates(struct 
> rev_cmdline_info *info)
>                * This ref will not be updated through a commit, lets make
>                * sure it gets properly updated eventually.
>                */
> -             if (commit->util || commit->object.flags & SHOWN)
> +             if (*revision_sources_at(&revision_sources, commit) ||
> +                 commit->object.flags & SHOWN)

Here it uses *slab_at() which makes more sense.

Reply via email to