Paul Tan <[email protected]> writes: > On Wed, Mar 4, 2015 at 7:01 AM, Junio C Hamano <[email protected]> wrote: >> Paul Tan <[email protected]> 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
