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

[Kurt Zeilenga <kurt@OpenLDAP.org>]

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