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
