Software Code Quality Of Apache Analyzed 442
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..."
Open Source versus Closed (Score:3, Informative)
Apache 2.1 does not yet exist (Score:5, Informative)
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 [reasoning.com] before drawing any sound conclusions.
Links to the Reports (no free reg required) (Score:2, Informative)
Here are the links to the Apache Open Source Inspection Report you requested:
Apache Defect Report: http://www.reasoning.com/pdf/Apache_Defect_Report. pdf
[reasoning.com] . pdf [reasoning.com]
Apache Metric Report: http://www.reasoning.com/pdf/Apache_Metric_Report
Reasoning provides the world's leading automated software inspection service. We boost the productivity of development teams by finding software defects faster and at a far lower cost than traditional approaches. Please let me know if you would like additional information. Thank you again for contacting Reasoning!
Sincerely,
Reasoning
Re:Apache 2.1 does not yet exist (Score:5, Informative)
The direct URLs for the reports are:
Defect Report [reasoning.com]
Metric Report [reasoning.com]
Re:Code defects appear to be a small part of the e (Score:4, Informative)
Re:No cigar, my ass. (Score:4, Informative)
5100 != 58,944
58,944 is the number from the article.
FACT: Reading is Good (Score:5, Informative)
Re:Defect? (Score:5, Informative)
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 (Score:5, Informative)
Some things I found interesting:
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.
Re:Wrong Math (Score:2, Informative)
The source of most free software [KDE is an exception] tend to be smaller, more readable and more effective. Ever wondred why winword.exe is 10.598.984 bytes?
Null dereferences and uninitialized variables (Score:3, Informative)
sorry, but thats pure BS... (Score:3, Informative)
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.
appache got its own malloc() that kills the child (and closes connection) if it fails to allocate enough bytes.
Re:Confuse with Linux? (Score:3, Informative)
Here are the links to the defect reports (Score:5, Informative)
Metric Report [reasoning.com]
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... (Score:5, Informative)
For instance, the first bug is
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.
This is a dupe (Score:3, Informative)
Re:sorry, but thats pure BS... (Score:2, Informative)
Is the idea that this code will never be executed in an out-of-memory condition, because it is only executed by a child, and the child dies automatically on malloc failure?
Re:So if they found them... (Score:2, Informative)
Eitherway I prefer
"--std=c99 -pedantic -Wall -W -Wshadow" as my warnings for GCC. It catches a shit-load of common coding foobars and also ensures the code follows ISO C [definite bonus].
Tom
Re:So if they found them... (Score:2, Informative)
It would be interesting to see how far 2.1 has progressed since then.
Re:So if they found them... (Score:5, Informative)
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:
This lets the compiler catch errors where you meant '==' rather than just '='. As in
Re:what is a "software error"? (Score:2, Informative)
BINGO (Score:3, Informative)
Apache has it's own malloc that kills the connection (and the child) if it fails.
That code can never be reached. Their test is invalid.
Re:Code defects appear to be a small part of the e (Score:4, Informative)
For example, I managed to code, test, and patch a "fix" for PostgreSQL this weekend in under 2 hours, having never seen the code before.
The "fix" wasn't a bug, per se, i't just that the output of pg_dump wasn't optimal in my usage for dumping the schema for CVS revision control. I added two flags, -m -M, which molded the output to my liking.
If you haven't seen your code in two months, you and an outsider have about the same chance at finding and detecting bugs/misfeatures.
Re:So if they found them... (Score:2, Informative)
Microsoft C++ catches this. Doesn't gcc? (Score:3, Informative)
MY compiler (Microsoft C++) does catch this
and issues a warning. Doesn't gcc?Re:So if they found them... (Score:3, Informative)
(In effect it does a lint-like check on the source.)
Re:Microsoft C++ catches this. Doesn't gcc? (Score:1, Informative)
GCC does not issue a warning for this, though:Note parenthesis.
Re:Microsoft C++ catches this. Doesn't gcc? (Score:4, Informative)
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....
Re:Interesting, with or without modules? (Score:2, Informative)
- All *.c files under httpd-2.0.46 - 375K lines
- APR (i.e. srclib) - 230K lines
- All modules - 93K lines
- modules/http - 5K lines
- modules/loggers - 1.6K lines
- modules/cache - 0.4K lines
- some files from modules/mappers - 4K lines
375 - 230 - 93 + 5 + 1.6 + 0.4 + 4 = 63K in ~ 100 filesSubtract 53 lines per file on Apache Software Licence and you'll end up with ~58K.
Re:So if they found them... (Score:2, Informative)
if (pointer)
is perfectly legal, and portable, C.
Re:what is a "software error"? (Score:1, Informative)
The first "defect" is provably not a defect at all (Score:3, Informative)
current_provider = conf->providers;
do {
{some safe code}
if (!conf->providers) {
break;
}
current_provider = current_provider->next;
} while (current_provider);
and they identify the second-to-last line as the "possible NULL pointer reference". Note that the "break" before that line will be taken if the pointer is NULL, so it can't happen. In fact, the static analysis could have determined this if it were a little better at propagating values.
First conclusion: subtract at least one "bug" from the 31 defects in Apache. This lowers the rate to 0.51, the same as the "average commercial code" number they quote. Yahoo!
Second conclusion: their static analysis must identify a lot of false positives, if the very first one in the list is one (I would look at more, but I should really get back to work...)
Re:So if they found them... (Score:2, Informative)
Funny you should point that out: a chained else-if structure without a terminal else is equivalent to a switch without a default which is notoriously vulnerable to the same sort of logic errors.
While I agree with that principle, the whole issue of good form (which I won't argue can be inefficient and cumbersome) is that following it slavishly can prevent the coding patterns that lead to hard-to-find bugs. It protects us from our own worst tendencies, one of which is assuming when we write the code that we know exactly what we mean it to do.
Re:So if they found them... (Score:2, Informative)
current_provider was assigned from conf->providers (line 257), so it cannot possibly be NULL unless conf->providers is NULL, and that condition is tested for on line 287.
NEXT!