On Fri, Aug 09, 2024 at 03:37:49PM +0000, John Cai via GitGitGadget wrote:
[snip]
> @@ -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
