From: Jeff Hostetler <git@jeffhostetler.com>
Date: Mon, 27 Nov 2017 19:39:43 GMT
Subject: Re: [PATCH v5 4/6] list-objects: filter objects in traverse_commit_list
Message-ID: <21fbd685-2c96-91b9-700e-8902a6e99c08@jeffhostetler.com>
In-Reply-To: <CAGZ79kZR-W-_JJi7zptiGgv=W7KpNMaSEbmmxrXcKovznK9E7Q@mail.gmail.com>



On 11/22/2017 5:56 PM, Stefan Beller wrote:
> On Tue, Nov 21, 2017 at 12:58 PM, Jeff Hostetler <git@jeffhostetler.com> wrote:
>> +       assert(arg);
>> +       assert(!unset);
> 
> I count 16 asserts in this patch. Is that really needed?
> Either omit them or use BUG if we want to rely on user
> bug reports when these conditions trigger, as assert is unreliable
> due to its dependence on the NDEBUG flag.

Yes, there are a few asserts in the code.  Old habits....

I could remove some/all of them, but personally I feel they
have merit and hint to the mindset of the author for future
readers of the code.  Are there other opinions?

Personally, I think it might be awkward to keep repeating
something like:

     if (!c)
         BUG(msg);

Do we want to think about a macro that builds on BUG() and
does the test?

Something like:
     #define ASSERT_OR_BUG(c) do { if (!(c)) BUG("%s", #c); } while (0)

Jeff