Re: [PATCH v5 1/5] setup: add gentle version of read_gitfile

[Jonathan Nieder <jrnieder@gmail.com>]

Jeff King wrote:
> On Tue, Apr 28, 2015 at 01:42:13PM -0700, Jonathan Nieder wrote:
>> Jeff King wrote:

>>> But the NULL does not carry the information about _which_ error, and
>>> Erik is suggesting that the caller may need to change behavior based on
>>> that information. IOW, his current patch (return NULL and set the
>>> specific integer code in a variable) allows this, but switching the
>>> integer code out for a human-readable strbuf does not.
>>
>> Right.  Two ways to handle that are:
>
> Sure, but "this system" that I was referring to one was not one of
> those ways. :)

That means I wasn't communicating well.

I never advocated *not* providing an integer return code when you want
to affect control flow.

An example of a function that provides information both to the caller
and user and doesn't conflate the two is ref_transaction_commit.  It
returns TRANSACTION_NAME_CONFLICT for a certain kind of recoverable
error and TRANSACTION_GENERIC_ERROR for all other errors.  In either
case, the 'err' parameter is populated with information intended for
the user.

Sometimes you might want to affect control flow without providing
information for the user.  Then there's no need for a string output
parameter and you can just provide the int.

At other times you might want to provide information for the user
without affecting control flow.  Then you can provide a string without
the int.

Providing error codes when you also want to return a string or struct
pointer is an unrelated problem.  One runs into that problem even if
only intending to provide a return code.  Possible approaches, like
mentioned before, are

 - an int * parameter to pass back the return code
 - using ERR_PTR to embed error codes in the pointer space

There is another approach which is sometimes nicer:

 - return int and pass the string or struct pointer through an output
   paramter

Thanks and sorry for the lack of clarity,
Jonathan