Re: [PATCH v4 1/3] refs: keep track of unresolved reference value in iterators

[shejialuo <shejialuo@gmail.com>]

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