All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed

From: Jeff King <peff@peff.net>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Michael Haggerty <mhagger@alum.mit.edu>
Subject: Re: [PATCH] xdiff: use BUG(...), not xdl_bug(...)
Date: Mon, 7 Jun 2021 15:44:03 -0400	[thread overview]
Message-ID: <YL53AylIeN+YdOTA@coredump.intra.peff.net> (raw)
In-Reply-To: <patch-1.1-68bf1ba4d3-20210607T164305Z-avarab@gmail.com>

On Mon, Jun 07, 2021 at 06:43:49PM +0200, Ævar Arnfjörð Bjarmason wrote:

> The xdl_bug() function was introduced in
> e8adf23d1e (xdl_change_compact(): introduce the concept of a change
> group, 2016-08-22), let's use our usual BUG() function instead.
> 
> We'll now have meaningful line numbers if we encounter bugs in xdiff,
> and less code duplication.

Thanks, I think this is a good direction.

Back when that commit was originally done, we didn't include
git-compat-util.h in xdiff at all, so we couldn't rely on Git-specific
code (though I think BUG() did not even exist back then anyway!).

These days I think we're comfortable with the notion that there is no
active upstream for xdiff, so we'd not be likely to send our changes
anywhere (and thus dependencies on our helpers are not a problem). But
there is one complication: libgit2 uses xdiff, too, and has pulled in
our changes including xdl_bug().

I wondered if we could make it easier on them by keeping the change in
one spot. But doing:

  static void xdl_bug(const char *msg)
  {
	BUG("%s", msg);
  }

is not really helpful, as it loses the line numbers. I guess we could
do:

  #ifdef BUG
  #define xdl_bug(msg) BUG(msg)
  #else
  ...the current implementation
  #endif

But really, libgit2 could just as easily define their own BUG() as they
see fit. Whether it is called xdl_bug() or BUG() in the call-sites is
not really that important.

So I think this patch is fine in its current form.

-Peff

     prev parent reply	other threads:[~2021-06-07 19:44 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-07 16:43 [PATCH] xdiff: use BUG(...), not xdl_bug(...) Ævar Arnfjörð Bjarmason
2021-06-07 19:44 ` Jeff King [this message]

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

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

  git send-email \
    --in-reply-to=YL53AylIeN+YdOTA@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    /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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.