Software Code Quality Of Apache Analyzed

fruey writes "Following Reasoning's February analysis of the Linux TCP/IP stack (putting it ahead of many commercial implementations for it's low error density), they recently pitted Apache 2.1 source code against commercial web server offerings, although they don't say which. Apparently, Apache is close, but no cigar..."

Comments

Apache 2.1 does not yet exist

[David McBride]

Umm, Apache 2.1 hasn't been released yet. Current latest stable is 2.0.46.

I can only assume that they're looking through the current DEVELOPMENT codebase -- finding a higher ``defect density'' in such a development codebase compared with commercial offerings is not exactly unexpected.

They're also some automated code inspection product; the press release doesn't go into details as to the severity of the defects found or the testing methodology.

It'll be necessary to read through the full report before drawing any sound conclusions.

Re:Apache 2.1 does not yet exist

[David McBride]

The above link wants your email address. Bah.

The direct URLs for the reports are:
Defect Report
Metric Report

Re:Code defects appear to be a small part of the e

[phre4k]

Prette lame when we are talking server software where apache has the lead. (apache 63% vs IIS 25% netcraft.com)

/Esben

FACT: Reading is Good

[Cancel]

That's not what they're saying at all. In fact, Reasoning concluded that there was no statistically significant difference in 'defect density' between Apache and the unnamed commercial product.

"In our February study that compared the defect density of the Linux TCP/IP stack to the average defect density of commercially developed TCP/IP stacks, we concluded that Open Source had a significantly lower defect density compared to commercial equivalents," said Bill Payne, President & CEO of Reasoning. "We received numerous inquiries about that study and took seriously requests for us to examine defect density rates in a less mature Open Source application and compare it with the commercial equivalent. Taking advantage of our database of automated software code inspection projects, we were able to do exactly that, and found the difference in defect density between the two was not significant." (emphasis mine)

Re:Defect?

[richie2000]

From the report:
NULL Pointer Dereference (Expression dereferences a NULL pointer) 29 instances
Uninitialized Variable (Variable is not initialized prior to use) 2 instances

They also list the files and code snippets where the errors were found.

In addition, the comparison is made against an industry average of commercial code they have tested this way, NOT against other webservers.

Defect Details

[Eustace Tilley]

Interested persons can download the full defect report free of charge.

Some things I found interesting:

1. Apache 2.1 (dev) is a mere 76,208 LOC.
2. No memory leaks detected
3. 29 NULL pointer dereferences
4. 2 Uninitialized variables
5. No bounds errors, no bad deallocs
6. otherchild.c had a rate of 7 NULL pointer dereferences per 1000 KSLC
7. 
   
   One of the explanations (given by Reasoning) for a NULL pointer dereference is "can occur in low memory conditions," which I think means the original allocator did not check for malloc failure.
   
   So you can get a sense of what a defect looks like, here is #21. The orignal uses bold and fonts improve readability, but I don't know how to reproduce that in slashcode:
   DEFECT CLASS: Null Pointer Dereference
   
   DEFECT ID 21
   
   LOCATION: httpd-2.1/srclib/apr/misc/unix/otherchild.c : 137
   
   DESCRIPTION The local pointer variable cur, declared on line 126, and assigned on line 128, may
   be NULL where it is dereferenced on line 137.
   PRECONDITIONS The conditional expression (cur) on line 129 evaluates to false.

   CODE FRAGMENT
   124 APR_DECLARE(void) apr_proc_other_child_unregister(void *data)
   125 {
   126 apr_other_child_rec_t *cur;
   127
   128 cur = other_children;
   129 while (cur) {
   130 if (cur->data == data) {
   131 break;
   132 }
   133 cur = cur->next;
   134 }
   135
   136 /* segfault if this function called with invalid parm */
   137 apr_pool_cleanup_kill(cur->p, cur->data, other_child_cleanup);
   138 other_child_cleanup(data);
   139 }

Here are the links to the defect reports

[arrogance]

Defect Report

Metric Report

They make you fill out a form that asks for your email and then do an opt out checkbox at the bottom of the form (you have to check it to NOT get spam from them). The site's a bit slashdotted right now though.

Re:So if they found them...

[Jeremy Erwin]

If you download the defect report (available from here*, it will explain exactly where the bugs are.
For instance, the first bug is

DEFECT CLASS: Null Pointer Dereference DEFECT ID 1
LOCATION: httpd-2.1/modules/aaa/mod_auth_basic.c :291
DESCRIPTION The local pointer variable current_provider, declared on line 235, and assigned on line 257, may be NULL where it is dereferenced on line 291.
PRECONDITIONS The conditional expression (res) on line 253 evaluates to false AND
The conditional expression (!current_provider) on line 264 evaluates to true AND
The conditional expression (!provider || !provider->check_password) on line 268
evaluates to false AND
The conditional expression (auth_result != AUTH_USER_NOT_FOUND) on line
282 evaluates to false AND
The conditional expression (!conf->providers) on line 287 evaluates to false.

Each bug report is followed by the snippet of source code containing the defect.

The metric report simply reports the statistics. For instance, the most bug ridden file is otherchild.c. The most common bug class is "dereferencing a NULL pointer".

If the Apache developers simply want to fix the bugs, they can use the Defect Report. If they want conduct a brutal purge of their contributors, they can use the Metric report.

*Yes, Reasoning wants an email address. They will mail you a URL (a rather simple one at that) to access the reports.

Re:So if they found them...

[Skjellifetti]

None of that bug report is at all useful if there is no logical way for all of those preconditions they listed to actually be met.

Well, Yes and No. The problem is that there may be no logical way that the pointer may be NULL today. But tomorrow, a new coder will add something that modifies the preconditions and suddenly that pointer can indeed be NULL. Even where you are sure that a condition is impossible, it is usually a good idea to check for NULL in order to avoid future errors.

And for those who haven't seen this trick before, a nice habit to get into is to write your checks like so:

if (NULL == myPointer) { ... }

This lets the compiler catch errors where you meant '==' rather than just '='. As in

/* Do we really mean this? */
if (myPointer = NULL) { ... }

Re:Microsoft C++ catches this. Doesn't gcc?

[Rasta Prefect]

This lets the compiler catch errors where you meant '==' rather than just '='.
MY compiler (Microsoft C++) does catch this

if (myPointer = NULL) { ... }
and issues a warning. Doesn't gcc?

Yes, it does. So does every other C compiler I've ever used (quite a few). I suspect the original poster may be the sort who ignores warnings....