Re: [PATCH v4 1/3] refs: keep track of unresolved reference value in iterators
From: shejialuo <hidden>
Date: 2024-11-23 08:24:08
On Fri, Aug 09, 2024 at 03:37:49PM +0000, John Cai via GitGitGadget wrote: [snip]
quoted hunk ↗ jump to hunk
@@ -66,6 +69,7 @@ static void free_ref_entry(struct ref_entry *entry) */ clear_ref_dir(&entry->u.subdir); } + free(entry->u.value.referent); free(entry); }
Today, I am learning the source code of the "ref-cache.[ch]". I feel
rather confused here. And I think this usage is wrong.
"free_ref_entry" will do the following things:
1. If "entry" is a directory, it will call "clear_ref_dir" which will
call "free_ref_entry" for every loose ref.
2. If "entry" is a loose ref, it will call `free(entry->u.value.referent)`
and `free(entry)`.
The problem is if "entry" is a directory, we will also execute the
following statement:
free(entry->u.value.referent);
This does not make sense. We should never access the "entry->u.value" if
"entry" is a directory. So, I think the correct usage should be:
if (entry->flag & REF_DIR) {
...
clear_ref_dir(...);
} else {
free(entry->u.value.referent);
}
Thanks,
Jialuo