lwIP - A Lightweight TCP/IP stack - Tasks: task #6792, Create ASSERTs for DEBUG compile
You are not allowed to post comments on this tracker with your current authentication level.
task #6792: Create ASSERTs for DEBUG compile
Submitter: | Simon Goldschmidt <goldsimon> | ||
Submitted: | Fri 20 Apr 2007 07:06:55 PM UTC | ||
Category: | None | Should Start On: | Fri 20 Apr 2007 12:00:00 AM UTC |
Should be Finished on: | Fri 20 Apr 2007 12:00:00 AM UTC | Priority: | 1 - Later |
Status: | Cancelled | Privacy: | Public |
Assigned to: | None | Percent Complete: | 0% |
Open/Closed: | Closed | Planned Release: | None |
Effort: | 0.00 |
Jump to the original submission
Thu 24 May 2007 07:20:42 AM UTC, comment #17: |
Simon Goldschmidt <goldsimon> |
Wed 23 May 2007 05:20:28 PM UTC, comment #16:
|
Simon Goldschmidt <goldsimon> |
Wed 23 May 2007 04:37:49 PM UTC, comment #15: 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 e.g. mem_malloc
|
Jonathan Larmour <jifl> |
Wed 23 May 2007 04:07:45 PM UTC, comment #14:
|
Simon Goldschmidt <goldsimon> |
Wed 23 May 2007 03:13:07 PM UTC, comment #13: I'm still of the opinion that asserts are a debug feature, and so all the existing LWIP_ASSERTs would remain as LWIP_ASSERTs.
|
Kieran Mansley <kieranm> |
Wed 23 May 2007 03:05:44 PM UTC, comment #12:
|
Simon Goldschmidt <goldsimon> |
Wed 23 May 2007 03:01:03 PM UTC, comment #11: 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?
|
Jonathan Larmour <jifl> |
Wed 23 May 2007 02:54:02 PM UTC, comment #10: I can see I wasn't very clear. What I'm proposing is:
|
Kieran Mansley <kieranm> |
Mon 14 May 2007 10:34:01 AM UTC, comment #9:
|
Simon Goldschmidt <goldsimon> |
Mon 30 Apr 2007 01:56:40 PM UTC, comment #8: 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:
|
Kieran Mansley <kieranm> |
Mon 30 Apr 2007 01:48:26 PM UTC, comment #7: 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 difference.
|
Jonathan Larmour <jifl> |
Mon 30 Apr 2007 01:32:04 PM UTC, comment #6: 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.
|
Simon Goldschmidt <goldsimon> |
Mon 30 Apr 2007 01:21:30 PM UTC, comment #5: 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.
|
Jonathan Larmour <jifl> |
Mon 30 Apr 2007 11:11:25 AM UTC, comment #4: 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.
|
Kieran Mansley <kieranm> |
Sun 29 Apr 2007 04:35:02 PM UTC, comment #3: I added LWIP_DEBUG_ASSERT(x,y) to debug.h in CVS HEAD. This triggers #ifndef LWIP_NOASSERT and #ifdef LWIP_DEBUG.
|
Simon Goldschmidt <goldsimon> |
Fri 27 Apr 2007 07:32:26 PM UTC, comment #2:
|
Simon Goldschmidt <goldsimon> |
Mon 23 Apr 2007 11:54:33 AM UTC, comment #1: 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. |
Kieran Mansley <kieranm> |
Fri 20 Apr 2007 07:06:55 PM UTC, original submission:
I've read on a different thread that we don't take much care in checking application error conditions (wrong arguments etc.)?
|
Simon Goldschmidt <goldsimon> |
No files currently attached
Depends on the following items: None found
Items that depend on this one: None found
There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.
Follow 2 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2007-05-24 | goldsimon | Open/Closed | Open | Closed | |
2007-05-24 | goldsimon | Status | None | Cancelled |
Powered by Savane 3.13-d3ae.
Corresponding source code
I removed LWIP_DEBUG_ASSERT again.
Closing this as cancelled.
I'm opening a new task to review all uses of ASSERT to maybe return error values.