Re: [PATCH v4 4/7] bulk-checkin: implement `SOURCE_INCORE` mode for `bulk_checkin_source`

From: Jeff King <peff@peff.net>
Date: Oct 23, 2023, 18:58 UTC
Message-ID: <20231023185842.GE1537181@coredump.intra.peff.net>
In-Reply-To: <ZTY6kTZT-ni16usH@tanuki>

On Mon, Oct 23, 2023 at 11:19:13AM +0200, Patrick Steinhardt wrote:

Show 8 quoted lines

> > +	case SOURCE_INCORE:
> > +		assert(source->read <= source->size);
> 
> Is there any guideline around when to use `assert()` vs `BUG()`? I think
> that this assertion here is quite critical, because when it does not
> hold we can end up performing out-of-bounds reads and writes. But as
> asserts are typically missing in non-debug builds, this safeguard would
> not do anything for our end users, right?

I don't think we have a written guideline. My philosophy is: always use BUG(), because you will never be surprised that the assertion was not compiled in (and I think compiling without assertions is almost certainly premature optimization, especially given the way we tend to use them).

-Peff