On Sat, Feb 01, 2025 at 10:33:13AM +0000, Phillip Wood wrote:

quoted

In normal production code, we'd expect something like:

   if (err)
	return -1;

to avoid running the rest of the function after the first error. But the
test harness check() function doesn't return. It just complains to
stdout and keeps running!

That is to allow the test to add more context with test_msg() or do things
like check all the members of a struct before returning. It is a bug in the
test if it does not return after finding a NULL pointer, the correct usage
is

	if (!check(ptr))
		return;

As we're in the process of switching to using clar which does exit the text
function if a check fails (that means there may be leaks on failure but if
the test is failing then I don't think we should be worrying about leaks) I
don't know if it is worth fixing these or not. I guess it depends if there
are the list of targets for Seyi's Outreachy project.

Ah, that's good to hear. I don't think there's any urgency here. These
have been popping up since people started adding more unit-tests/ last
summer. Waiting a few more months to switch to clar is probably not a
big deal.

I'm OK with ignoring a leak in a failing test. I do suspect that
Coverity might still complain about the leaks, because it is doing
static analysis to show that we _can_ leak (rather than the tests, which
are seeing if we leaked at runtime). But I'm not sure how much effort we
want to spend on making tests do cleanup on failure. Especially in a
language like C.

Anyway, I'll continue to ignore these Coverity results for now, then. ;)

-Peff