Re: [PATCH 3/3] fixup! reftable: add a heap-based priority queue for reftable records

From: Jeff King <peff@peff.net>
Date: Sep 2, 2021, 09:09 UTC
Message-ID: <YTCU3F+0sKcrym6F@coredump.intra.peff.net>
In-Reply-To: <20210902053023.44006-4-carenas@gmail.com>

On Wed, Sep 01, 2021 at 10:30:23PM -0700, Carlo Marcelo Arenas Belón wrote:

Show 17 quoted lines

> diff --git a/reftable/pq.c b/reftable/pq.c
> index 8918d158e2..a6acca006b 100644
> --- a/reftable/pq.c
> +++ b/reftable/pq.c
> @@ -43,12 +43,14 @@ int merged_iter_pqueue_is_empty(struct merged_iter_pqueue pq)
>  
>  void merged_iter_pqueue_check(struct merged_iter_pqueue pq)
>  {
> +#ifndef NDEBUG
>  	int i = 0;
>  	for (i = 1; i < pq.len; i++) {
>  		int parent = (i - 1) / 2;
>  
>  		assert(pq_less(pq.heap[parent], pq.heap[i]));
>  	}
> +#endif
>  }

This will trigger -Wunused-parameter warnings, since the function body is now empty when NDEBUG is undefined. Probably switching the assert() to die() would be better, since the whole point of the function is just to exit on error.

If there's a problem using die() from the reftable code, it could also return an error and the caller in the test helper could propagate it.

-Peff