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

[Junio C Hamano <gitster@pobox.com>]

Paul Tan <pyokagan@gmail.com> writes:

> On Wed, Mar 4, 2015 at 7:01 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Paul Tan <pyokagan@gmail.com> writes:
>>
>>>       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.