Summary: Create ASSERTs for DEBUG compile
Project: lwIP - A Lightweight TCP/IP stack
Submitted by: goldsimon
Submitted on: Freitag 20.04.2007 um 19:06
Should Start On: Freitag 20.04.2007 um 00:00
Should be Finished on: Freitag 20.04.2007 um 00:00
Priority: 1 - Later
Percent Complete: 0%
Assigned to: None
Discussion Lock: Any
I've read on a different thread that we don't take much care in checking
application error conditions (wrong arguments etc.)?
This probably comes from fear of performance loss, which I can understand.
This could make porting / learning lwIP a hard time.
As a solution, I propose to add a new assert macro, LWIP_DEBUG_ASSERT() that
only executes if LWIP_DEBUG is defined. That way we can avoid performance
loss due to error checking and still can discover misbehaviour (mostly of the
programmer) during creation/debugging phase. Currently, we can only turn on or
off all asserts together and can't decide between critical and
Asserts are a very useful tool, and this is definitely good. One thing I
would request is that whenever you add an assert you think about if you also
need to add something to cope if the assert doesn't fire (i.e. on a not-DEBUG
build). Too often people add asserts and forget that they won't always be
there. Sometimes (if they are asserting against programmer error or API
abuse) that's OK, but other times dealing with it gracefully in a release
build is possible too.
I would argue that asserts are a debug-only tool, and that they should
compile out with no side effects (other than the assertion not being there)
by default, in the same way that the LWIP_DEBUGF macros do.
In theory, there should be no fatal errors in the release code! However, in
practice I can see that that won't always be true, but in such situations
people can turn on the debug macros to get the assertions and find out what
the problem is.
I think some documentation and clarity is required for what the distinction
between these two types of assert is. I'm still not clear on this.
Also developers need to know somewhere or other about the effect of this?
Perhaps LWIP_NOASSERT could be mentioned in opt.h, along with a new define
in there for LWIP_DEBUG, which surprisingly is not already in there, despite
being quite an important option IMHO.
I wanted to introduce LWIP_DEBUG_ASSERT() to have an easy way of adding extra
checks when developing which should normally not fire. This could be an
lwIP-newbie for example, who needs some extra-checks.
Or a developer like me, who wants to make sure the code works using some
ASSERTs not really needed after the code works. In this case, I think having
DEBUG-ASSERTs is better than using normal ASSERTs and removing them later, as
you have kind of documentation of the original programmer's idea/checks in
the code. I think that alone is worth a lot looking at some of the
discussions we had throughout the last months.
On the other hand, I think it sometimes is easier to run into an ASSERT
(which for me, automatically reboots the system through a watchdog) than
including code for all kind of failures. So I think there should be a
distiction between normal ASSERTs (triggering on errors that can happen
during normal running of an application) and DEBUG-ASSERTs (triggering if you
use the stack in a wrong way). The latter can be turned off once you know your
code/configuration is OK.
I think I see what you're getting at. Perhaps there might be a clearer way to
put this in the name - for me asserts are always a "debug" thing, so having
one be LWIP_ASSERT and another LWIP_DEBUG_ASSERT doesn't offer much
Just a suggestion, but perhaps there could be other names like
LWIP_APP_ASSERT, LWIP_USER_ASSERT, LWIP_API_ASSERT?
Anyway, thanks for the explanation. Perhaps there's some file (or in a
comment in opt.h) a summary of this could go into?
I agree that asserts are always debug. If you're trying to check for errors
in release code, asserts are not the way to do it - you should write an error
handler instead if the error is expected. However, I can see that others
might want them. I therefore propose:
LWIP_ASSERT for the debug variety that causes whatever is running to fail in
a non-graceful manner
LWIP_CHECK for the non-debug variety that gives a warning, but doesn't do the
In a debug build LWIP_CHECK could be compiled to LWIP_ASSERT
>In a debug build LWIP_CHECK could be compiled to LWIP_ASSERT
But in a release build both will be left out? That's OK (and faster) if we
make sure that all errors are catched through normal program flow / return
values. We can then remove LWIP_DEBUG_ASSERT and also are faster than now.
That would be a new task, I think.
I can see I wasn't very clear. What I'm proposing is:
In a release build:
- LWIP_ASSERT would compile out (as they're only really there to catch
- LWIP_CHECK would be a non-fatal warning that something is wrong (as
they're checks for things that could potentially go wrong in real world
In a debug build:
- LWIP_ASSERT and LWIP_CHECK would both be fatal assertions
So it seems you're suggesting that LWIP_ASSERT would then correspond with
what Simon proposed as LWIP_DEBUG_ASSERT, and many of the existing
LWIP_ASSERTs in current lwIP would become LWIP_CHECK?
Or asking it another way, would LWIP_CHECK be checking for things that really
can go wrong sometimes, or only go wrong if there's a bug in lwIP? It seems to
me that the former is better handled with error returns. So I guess you mean
>Or asking it another way, would LWIP_CHECK be checking for things that
really can go wrong sometimes, or only go wrong if there's a bug in lwIP? It
seems to me that the former is better handled with error returns. So I guess
you mean the latter?
I'd also like to disable both to have faster code in a bug-free lwIP stack
;-) But maybe it is better to have some checks all the time...
And if you mean LWIP_CHECK should fire for a bug in lwip, I'd call it
LWIP_FATAL. I don't really understand the meaning of LWIP_CHECK. If it's a
non-fatal check, why include it in a release build?
I'm still of the opinion that asserts are a debug feature, and so all the
existing LWIP_ASSERTs would remain as LWIP_ASSERTs.
The LWIP_CHECK was suggested because I thought there was a wish to have
something akin to an assert that persisted in a release build. I was keen
that this not actually be a fatal error, as such things in release builds are
bad. I don't have a good example of what it would be used for (and agree that
error handlers would be in most cases better) but there seemed to be a demand
for this, although perhaps I've misunderstood. Perhaps for those cases where
the programmer is assuming something to be true, and really thinks it will
always be true, but wants a little insurance that there will be some
notification if it turns out not to be true. They could use LWIP_CHECK to
get this insurance.
I would also be OK with no LWIP_CHECK if people prefer that. Or with
LWIP_ASSERT becoming just a warning in a release build.
I disagree about NULL pointer checks in a release build. That would be a
programmer error. Unless you are referring to return values from calls to
Otherwise, code should be debugged until it works. In most cases in embedded
stuff, if it doesn't work in the field, there's nothing you can do anyway.
What's the point of having an assert in a TV? Or an air conditioning unit?
(Yes those are two examples where customers of ours are using TCP/IP
stacks!). Or even a printer. In the field there's no point - use a watchdog
instead and just reset. Or if it's that important to you, then leave asserts
on in your release build. But for those who write programs that work, these
sorts of checks are a pointless waste of code.
But to close this topic: I will remove the DEBUG_ASSERT again and we'll make
sure to change the code in such way that there are ASSERTs for programmer
errors and error return values for errors that can happen (that's not always
the case until now!).