[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: Saturn v1.1 on openldap v2.4.4alpha

Again, 2.4.4alpha is quite old. It's generally recommend anyone having issue with it try either use HEAD or RE23. Given the nature of your report, HEAD would be far more appropriate. This report contains items concerning code, for instance slurpd(8), that been axed from HEAD and won't appear in the upcoming 2.4 beta. It also doesn't cover codes that will be new in 2.4 beta.

On Aug 22, 2007, at 2:19 AM, Domagoj Babic wrote:
I'd appreciate if anyone could provide feedback on this report from
another tool. I've filtered out all the warnings that are provably false.

I've selected a couple of items to comment on...

The first one I examined complains that a pointer returned ch_malloc () and dereferenced could be NULL. ch_malloc() obviously cannot possibly return NULL (a fact your tool apparently missed). Upon malloc failure, ch_malloc will either abort(3) (the usual behavior) or exit(3) as expected. As I noted in my initial response to your prior report, in OpenLDAP Software code, it is generally expected that a malloc failure will result in a either an abort(3) or a crash (by dereferencing NULL). This is especially so in slapd(8) (and not especially true in libldap/liblber).

The second one I examined complains about an inconsistency in a routine. A pointer argument to the routine was dereferenced before an assert(3) checking that the pointer was NULL. This is a minor inconsistency, the dereference should follow the assertion. However, this inconsistency doesn't cause the behavior of the routine to deviate from the expected (it still will crash as expected). The difference is that, if it were to crash on the assert, it's slightly more obvious to the developer that it crashed because the argument was NULL. Such inconsistencies crop up every now and then and are generally cleaned up when noticed. Thanks for noticing them.

While these asserts will fire on malloc failures, they are actually inserted to catch programming errors where a NULL (not returned by malloc) is passed into routine.

So, pointer assertions often serve one of two purposes, and some times both. As in ch_malloc(), it used to cause a sooner than later crash after a malloc() failure. As in libldap routines, it used to valid that the a pointer argument expected to be non-NULL is non-NULL instead of crashing on the deref (as is a common practice in standard C libraries). In many slapd(8) routines, some assertions may serve both purposes.

Anyways, neither of these two items are indicative of bugs in OpenLDAP Software. Here I will use the definition of software bug that I believe is widely accepted by this community:
A software bug (or just "bug") is an error, flaw, mistake, failure, or fault in a computer program that prevents it from behaving as intended (e.g., producing an incorrect result). [Wikipedia]
As the intended behavior is to crash, and slapd(8) will in both cases, neither is a bug (the second is an coding inconsistency). You're welcomed to use a different definition of bug for your purposes, that's your business.

You may find slapd(8) memory handling a bit odd. A lot of folks do. There are reasons for it, the details of which, if interested, you likely can find in the openldap-devel list archives. Regardless of the reasons for it, that's the way it is and it is very unlikely to change in this code base.

I note that this will likely be my first and only comment in this thread. I now need to attend to other things (like my day job).

-- Kurt