Re: [PATCH v2 2/4] run-command: use BUG() to report bugs, not die()

From: Jeff King <peff@peff.net>
Date: May 7, 2018, 09:08 UTC
Message-ID: <20180507090806.GB367@sigill.intra.peff.net>
In-Reply-To: <91ddc7ed5ce0279c5d48bbed595d932d985465c4.1525253892.git.johannes.schindelin@gmx.de>

On Wed, May 02, 2018 at 11:38:31AM +0200, Johannes Schindelin wrote:

Show 13 quoted lines

> 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