Re: Why do we have both x*() and *_or_die() for "do or die"?
- From
Stefan Beller <sbeller@google.com>
- Date
- May 22, 2018, 18:04 UTC
- Message-ID
- <CAGZ79kZs7aNXB2cn8p+m5gUOAv0OqhPW6b23y4cbm9d3Njwn=A@mail.gmail.com>
- In-Reply-To
- <874liz8tsi.fsf@evledraar.gmail.com>
On Tue, May 22, 2018 at 10:49 AM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
Show 5 quoted lines
> > 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