Re: [PATCH v2 1/4] worktree: refactor infer_backlink() to use *strbuf

[shejialuo <shejialuo@gmail.com>]

On Sun, Oct 06, 2024 at 02:41:22PM -0400, Eric Sunshine wrote:

[snip]

> > Why not simply do the following things here (I don't think
> > "git_contents" is a good name, however I am not familiar with the
> > worktree, I cannot give some advice here).
> 
> I found the name "git_contents" clear enough and understood its
> purpose at-a-glance, so I think it's a reasonably good choice. A
> slightly better name might be "gitfile_contents" or perhaps
> "dotgit_contents" for consistency with other similarly-named variables
> in this function.
> 

Thanks, I don't know the context here.

> > If "git_contents" is not
> > NULL, there is no need for us to check the "err" variable.
> 
> I'm not sure we would want to change this; the existing logic seems
> clear enough.
> 

The reason why I don't think we need to check the "err" variable is that
the "git_contents" and "err" is relevant. If "git_contents" is not NULL,
the "err" must be zero unless there are bugs in "read_gitfile_gently".
So, if we already check "git_contents", why do we need to check again
for "err"?

But, I agree with you that the existing logic is clear enough. I just
explain further what I mean here.

Thanks,
Jialuo