Re: [PATCH v6 3/9] ref: initialize target name outside of check functions

[shejialuo <shejialuo@gmail.com>]

On Tue, Nov 05, 2024 at 08:11:46AM +0100, Patrick Steinhardt wrote:
> On Mon, Oct 21, 2024 at 09:34:31PM +0800, shejialuo wrote:
> > We passes "refs_check_dir" to the "files_fsck_refs_name" function which
> > allows it to create the checked ref name later. However, when we
> > introduce a new check function, we have to re-calculate the target name.
> > It's bad for us to do repeat calculation. Instead, we should calculate
> > it only once and pass the target name to the check functions.
> 
> It would be nice to clarify what exactly is bad about it. Does it create
> extra memory churn? Or is this about not duplicating logic?
> 

Thanks, I will improve this in the next version.

> > In order not to do repeat calculation, rename "refs_check_dir" to
> > "target_name". And in "files_fsck_refs_dir", create a new strbuf
> > "target_name", thus whenever we handle a new target, calculate the
> > name and call the check functions one by one.
> 
> "target_name" is somewhat of a weird name. I'd expect that this is
> either the path to the reference, in which case I'd call this "path", or
> the name of the reference that is to be checked, in which case I'd call
> this "refname".
> 

I felt quite hard to name this variable when I wrote the code. "refname"
is not suitable due to we may check the reflog later by calling
"files_fsck_refs_dir" function.

So, we should use "path" here.

Thanks,
Jialuo