Re: [PATCH 1/2] git-credential-store: support XDG config dir

Paul Tan [off-list ref] writes:

On Wed, Mar 4, 2015 at 7:01 AM, Junio C Hamano [off-list ref] wrote:

quoted

Paul Tan [off-list ref] writes:

quoted

      struct credential entry = CREDENTIAL_INIT;
+     int found_credential = 0;

      fh = fopen(fn, "r");
      if (!fh) {
              if (errno != ENOENT)
                      die_errno("unable to open %s", fn);
-             return;
+             return 0;

Returning found_credential here would be easier to read, no?  After
all, that is why you explicitly initialized it to 0 up there to say
"no we haven't found any yet".

Actually I think die_errno is a function that does not return at all.
The return is just to shut the compiler up. Perhaps I shall comment
that.

Commenting just on this part (I am not agreeing or disagreeing with
you on other parts of your message yet).

When fopen() fails because we cannot open an existing file for
reading, then die_errno() will trigger and we stop there, in which
case the return will not be reached.

But when we try to open fn and we fail only because fn does not
exist, we do not say "die".  We instead return to the caller,
telling it that we have not found any credential so far in the file
supplied by the caller.

So the return does matter, and spelling that zero with
found_credential does matter for readability, I would think.