Re: [PATCH v2 08/14] revision.c: use commit-slab for show_source
From: Junio C Hamano <hidden>
Date: 2018-05-14 05:10:22
Nguyễn Thái Ngọc Duy [off-list ref] 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);quoted hunk
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.