Re: [PATCH v2 2/4] run-command: use BUG() to report bugs, not die()
To
Johannes Schindelin
Cc
git@vger.kernel.org
Junio C Hamano
Johannes Sixt
Nguyễn Thái Ngọc Duy
Eric Sunshine
From
Jeff King
See Also
Prev Ref 1 Ref 2
Date
2018-05-07 09:08:06 UTC
On Wed, May 02, 2018 at 11:38:31AM +0200, Johannes Schindelin wrote:

> The slightly misleading name die_bug() of the function intended to
> report a bug is actually called always, and only reports a bug if the
> passed-in parameter `err` is non-zero.
> 
> It uses die_errno() to report the bug, to helpfully include the error
> message corresponding to `err`.
> 
> However, as these messages indicate bugs, we really should use BUG().
> And as BUG() is a macro to be able to report the exact file and line
> number, we need to convert die_bug() to a macro instead of only
> replacing the die_errno() by a call to BUG().
> 
> While at it, use a name more indicative of the purpose: CHECK_BUG().

Hrm, at first glance these _don't_ seem like BUGs, because we're
checking the value of errno. I guess the argument is that for these
_particular_ syscalls, we know that they can only complain with EINVAL,
and only when we feed them junk. And therefore any such error return is
a bug.

I guess I buy that line of reasoning for pthread_setcancelstate(). But I
think pthread_sigmask() could return an error based on in-kernel
limitations of sigsetsize. I suppose that's pretty unlikely.

But what I'm suggesting is that these probably should not have been
die("BUG") in the first place. And that the rule of thumb should
probably be that anything relying on system calls or errno should not
BUG(), but be a regular die().

-Peff