git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Johannes Schindelin <johannes.schindelin@gmx.de>
Cc: git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
	"Johannes Sixt" <j6t@kdbg.org>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>
Subject: Re: [PATCH v2 2/4] run-command: use BUG() to report bugs, not die()
Date: Mon, 7 May 2018 05:08:06 -0400	[thread overview]
Message-ID: <20180507090806.GB367@sigill.intra.peff.net> (raw)
In-Reply-To: <91ddc7ed5ce0279c5d48bbed595d932d985465c4.1525253892.git.johannes.schindelin@gmx.de>

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

  reply	other threads:[~2018-05-07  9:08 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-12 14:19 [Git 2.13.0] BUG: setup_git_env called without repository Josh Hagins
2017-05-12 14:48 ` Johannes Schindelin
2017-05-15  0:22   ` Josh Hagins
2017-05-12 20:34 ` [PATCH] config: complain about --local outside of a git repo Jeff King
2017-05-12 22:31   ` Ævar Arnfjörð Bjarmason
2017-05-13  2:03     ` Jeff King
2017-05-13  3:24       ` [PATCH 0/3] BUG() and "config --local" outside of repo Jeff King
2017-05-13  3:28         ` [PATCH 1/3] usage.c: add BUG() function Jeff King
2017-05-13  3:55           ` Jeff King
2017-05-15  2:28             ` Junio C Hamano
2017-05-13  3:29         ` [PATCH 2/3] setup_git_env: convert die("BUG") to BUG() Jeff King
2017-05-13  3:29         ` [PATCH 3/3] config: complain about --local outside of a git repo Jeff King
2018-05-02  9:38         ` [PATCH v2 0/4] Finish the conversion from die("BUG: ...") to BUG() Johannes Schindelin
2018-05-02  9:38           ` [PATCH v2 1/4] test-tool: help verifying BUG() code paths Johannes Schindelin
2018-05-02 15:18             ` Duy Nguyen
2018-05-05 19:30               ` Johannes Schindelin
2018-05-02  9:38           ` [PATCH v2 2/4] run-command: use BUG() to report bugs, not die() Johannes Schindelin
2018-05-07  9:08             ` Jeff King [this message]
2018-05-02  9:38           ` [PATCH v2 3/4] Replace all die("BUG: ...") calls by BUG() ones Johannes Schindelin
2018-05-02  9:38           ` [PATCH v2 4/4] Convert remaining die*(BUG) messages Johannes Schindelin
2018-05-07  9:01           ` [PATCH v2 0/4] Finish the conversion from die("BUG: ...") to BUG() Jeff King
2017-05-13  0:04   ` [PATCH] config: complain about --local outside of a git repo Jonathan Nieder
2017-05-13  2:04     ` Jeff King
2017-05-15  0:31   ` Josh Hagins
2017-05-15  3:18     ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180507090806.GB367@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=pclouds@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).