On Wed, 09 Aug 2006 17:04:13 -0700 Wes wrote: WH> Doing it as an assert is wrong.
You didn't really read my response, did you? > But it wasn't an assert, it was a netsnmp_assert. And if you look up the > definition of that macro (library/snmp_assert.h), it will only log a message > by default. The assert (segfault) will only happen if explicity enabled. > There's not even a configure flag to enable it, it has to be manually added > or explicity set in CFLAGS. So, if dave saw an assert, it's because he asked for it. If he saw a crash, then the default logging part of the netsnmp_assert macro might need tweaking. WH> Especially since the code right above WH> it makes the assert always trigger. I stared at that code for 5 WH> minutes this morning (before I did an update and happily noticed it WH> was gone) and couldn't figure out why the heck someone had written it WH> that way. A minimum it should have a comment above it saying you WH> expect it always to hit. Ok, i'm fine w/a comment that it's always expected to hit. WH> [I'm very tempted to get on my assert soapbox saying that all the WH> asserts in net-snmp are bad and evil because they're doing nothing WH> more than saying "I refuse to more error checking than letting you WH> know I know it'll be an error but I refuse to properly bail out of WH> it". But I won't go on that soap box. No not me. nope.] [ And I'd be tempted to respond along the following lines. I think they are very useful for several reasons, in various difference cases. The above case is/was a netsnmp_assert *in addition* to error handling. This is useful for a case where a particular error case really shouldn't happen. Silently handling it can mask a larger problem. Logging it (the current default for netsnmp_assert) will at least, hopefully, get us some bug reports/email messages asking us about it. How often do you check your log files to look for error messages that indicate a bug? And I have full asserts turned on in all my builds, so I'm damn well gonna notice it. I actually think turning on the assert behaviour should be the default in developer builds. (Yeah, yeah, I know, never gonna happen.) The other case where I like netsnmp_asserts is anyplace in the code that I happen to run across that doesn't have error handling, or where I think a certain condition would cause bad things to happen. Often this will happen in code that I'm not very familiar with, or while I'm working on something else and don't have time for a detailed investigation. A netsnmp_assert will let me know if the condition happens in practice, along with a backtrace for how the condition was arrived at. The other option, which I guess is what everyone else does, is either ignore the issues and hope the current code is good, or add a comment like 'xxx fix this'. There are *over 600* such comments in the code base. I haven't looked, but I'm guessing most of them are not accompanied by log messages. I'm also guessing that number is not going to shrink any time soon. Turn them all into netsnmp_asserts, and we're quickly going to find out which ones are being hit and need to be fixed. And knowing that others aren't getting hit would provide me with more confidence than a simple comment in the code. I will readily admit that when writing new code, any form of assert in lieu of proper error handling is a bad idea. But when dealing with a big mess of legacy code that you didn't write, might not totally understand and don't really have time to fully investigate and fix, they can help determine what problems are actually happening, and thus need to be fixed.] You find me funding to do proper error handling for the current cases of asserts, and I'll gladly get rid of them all. ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 _______________________________________________ Net-snmp-coders mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/net-snmp-coders
