On Mon, Oct 07, 2024 at 04:01:43AM +0000, Caleb White wrote:
> On Sunday, October 6th, 2024 at 22:56, shejialuo <shejialuo@xxxxxxxxx> wrote:
> > 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"?
>
> There are two other error conditions we check, and one of them we try
> to find the inferred backlink (so it is not a failure path):
>
> ```
> } else if (err == READ_GITFILE_ERR_NOT_A_FILE) {
> fn(1, realdotgit.buf, _("unable to locate repository; .git is not a file"), cb_data);
> goto done;
> } else if (err == READ_GITFILE_ERR_NOT_A_REPO) {
> if (!infer_backlink(realdotgit.buf, &backlink)) {
> fn(1, realdotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data);
> goto done;
> }
> } else if (err) {
> fn(1, realdotgit.buf, _("unable to locate repository; .git file broken"), cb_data);
> goto done;
> }
>
> ```
I am sorry that my words make you not clear here. I want to express that
if "git_contents" is not NULL and there is no need for us to check
"err". This means we could use the following flows:
if (git_contents && !err) {
...
} else if (err == xxx) {
...
}
However, from my perspective, the way proposed by Eric where we could
use "BUG" is more robust. Because the current method assumes that
"read_gitfile_gently" works as we want.
Thanks,
Jialuo
