On Tue, May 22, 2018 at 10:49 AM, Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> wrote:
>
> On Wed, May 16 2018, Stefan Beller wrote:
>
>> A common pattern with the repo_read_index function is to die if the return
>> of repo_read_index is negative. Move this pattern into a function.
This was done organically, i.e. taking the smallest available step that yields
a better code base. There was definitely no stepping back or looking at the
big picture involved.
> Just a side-question unrelated to this patch per-se, why do we have both
> x*() and *_or_die() functions in the codebase?
Digging into the history, the x*() emerges from 112db553b0d (Shrink the
git binary a bit by avoiding unnecessary inline functions, 2008-06-22) and
is wrapping common patterns to reduce the size of the binary.
The or_die pattern comes from 7230e6d042a (Add write_or_die(),
a helper function, 2006-08-21). This was motivated as better code
("making additional error handling unnecessary", "replace two similar ones")
None of them really hint at the _die() as a problem in the
libification efforts which came later IIUC.
> I can't find any pattern
> for one or the other, e.g. we have both xopen() and then write_or_die(),
hah! Up to now I assumed the x* is system calls with write_or_die as an
exception. There is also a xwrite and write_in_full, so they are slightly
different in the way that xwrite covers only the system call and copes
with some common error patterns, whereas write_or_die uses write_in_full
which itself uses xwrite.
> so it's not a matter of x*() just being for syscalls and *_or_die()
> being for our own functions (also as e.g. strbuf uses x*(), not
> *_or_die()).
I would tend to argue that way: x* are strictly to system calls (no die()
in there), and the _die() functions are "hands off *no* error handling
needed"
> I'm not trying to litigate the difference and understand it could have
> just emerged organically. I'm just wondering if that's the full story or
> if one is preferred, or we prefer one or the other in some
> circumstances.
I think we'd prefer the x() for lib code and the _or_die code in builtin/
due to the nature of effort needed to get it right.
Thanks,
Stefan
