Re: [PATCH 0/5] usage API: add and use a bug() + BUG_if_bug()

From: Josh Steadmon <steadmon@google.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Bagas Sanjaya <bagasdotme@gmail.com>,
	Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
Subject: Re: [PATCH 0/5] usage API: add and use a bug() + BUG_if_bug()
Date: Fri, 27 May 2022 10:02:08 -0700	[thread overview]
Message-ID: <YpEEEAprJOXYj5VK@google.com> (raw)
In-Reply-To: <cover-0.5-00000000000-20220521T170939Z-avarab@gmail.com>

On 2022.05.21 19:14, Ævar Arnfjörð Bjarmason wrote:
> This series adds a bug() (lower-case) function to go along with
> BUG(). As seen in 2-5/5 this makes it much easier to handle the cases
> such as parse-options.c where we'd like to call BUG(), but would like
> to first exhaustively accumulate the N issues we spot before doing so,
> and not merely BUG() out on the first one.
> 
> I have more fixes for parse-options.c queued up on top of this
> locally, including a fix for one (tiny) recent-ish regression, but
> found that it was much easier to do so with this new API, as we'll now
> be able to freely use normal sprintf() formats in these cases, instead
> of xstrfmt() (where we'd also memory leak).
> 
> Ævar Arnfjörð Bjarmason (5):
>   usage.c: add a non-fatal bug() function to go with BUG()
>   parse-options.c: use new bug() API for optbug()
>   parse-options.c: use optbug() instead of BUG() "opts" check
>   receive-pack: use bug() and BUG_if_bug()
>   cache-tree.c: use bug() and BUG_if_bug()
> 
>  .../technical/api-error-handling.txt          | 17 +++++-
>  Documentation/technical/api-trace2.txt        |  4 +-
>  builtin/receive-pack.c                        | 16 +++---
>  cache-tree.c                                  |  7 ++-
>  git-compat-util.h                             | 12 +++++
>  parse-options.c                               | 50 +++++++++---------
>  t/helper/test-trace2.c                        | 21 +++++++-
>  t/t0210-trace2-normal.sh                      | 52 +++++++++++++++++++
>  trace2.c                                      |  6 +++
>  usage.c                                       | 30 +++++++++--
>  10 files changed, 165 insertions(+), 50 deletions(-)
> 
> -- 
> 2.36.1.960.g7a4e2fc85c9
> 

Thank you for the series! I am generally in favor and think this is a
useful change. There are a few minor comments that I'll leave on
individual patches.

Thanks,
Josh