Re: [PATCH v2 08/14] revision.c: use commit-slab for show_source
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 14, 2018, 05:10 UTC
- Message-ID
- <xmqqfu2uddqg.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20180513055208.17952-9-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@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);Show 8 quoted lines
> 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.