Thread (13 messages) flat view 13 messages, 4 authors, 2016-12-22

Re: [PATCH] mailinfo.c: move side-effects outside of assert

From: Jeff King <hidden>
Date: 2016-12-19 20:03:58

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 <redacted>
---

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help