On Sat, Dec 17, 2016 at 11:54:18AM -0800, Kyle J. McKay wrote:

> Since 6b4b013f18 (mailinfo: handle in-body header continuations,
> 2016-09-20, v2.11.0) mailinfo.c has contained new code with an
> assert of the form:
> 
>       assert(call_a_function(...))
> 
> The function in question, check_header, has side effects.  This
> means that when NDEBUG is defined during a release build the
> function call is omitted entirely, the side effects do not
> take place and tests (fortunately) start failing.
> 
> Move the function call outside of the assert and assert on
> the result of the function call instead so that the code
> still works properly in a release build and passes the tests.
> 
> Signed-off-by: Kyle J. McKay <mack...@gmail.com>
> ---
> 
> Notes:
>     Please include this PATCH in 2.11.x maint

This is obviously an improvement, but it makes me wonder if we should be
doing:

  if (!check_header(mi, &mi->inbody_header_accum, mi->s_hdr_data))
        die("BUG: some explanation of why this can never happen");

which perhaps documents the intended assumptions more clearly. A comment
regarding the side effects might also be helpful.

-Peff

Reply via email to