This is the first in a series of snippets that present code that I personally consider an affront to humanity. All code must be from real, live projects. If the code is proprietary in origin, or otherwise confidential, symbol names might be changed to protect the guilty, but it’s ALL REAL. YMMV.
Today’s crime against humanity:
Returning from a within a macro
Witness this innocent chunk of code, from fs/intermezzo/journal.c in a kernel tree near you:
#define BUFF_ALLOC(newbuf, oldbuf) \
PRESTO_ALLOC(newbuf, PAGE_SIZE); \
if ( !newbuf ) { \
if (oldbuf) \
BUFF_FREE(oldbuf); \
return -ENOMEM; \
}
Do I really need to explain why returning from a macro is a bad, bad, bad, god awful idea? consider:
void foo(void)
{
void* buf1, buf2, buf3;
BUFF_ALLOC(buf1, NULL);
BUFF_ALLOC(buf2, buf1);
BUFF_ALLOC(buf3, buf2); <----- BOOOM. Bye bye buf1 memory if
allocation fails.
}
Resource allocation and deallocation should always be handled with care. Sequence points where resource book keeping is required must be explicit.
Muli Ben-Yehuda's journal 
dos and don’ts
I did tell you one of the talks I wrote (others gave it for me) is dos and don’ts in Python, right?
People make crappy code in all languages…
Comment by moshez — July 3, 2003 @ 12:19 AM |
Re: happy happy joy joy
I know, believe me, I know. Code That Sucks is an effort to maintain my sanity, considering some of the crap^H^H^H^Hcode that I see, by pointing out just why it’s crap.
Comment by mulix — July 3, 2003 @ 12:28 AM |
Code that Sucks ™ – possible in every environments…
Oh. Wrong mental model bug. Assume that X is Y, then screw up X with f, under (usually wrong) assumption that f(Y) would do the right thing.
Same shit as doing System.exit() in exception handler in Java (hint: if you are running under application server, such as JBoss, the server will exit, not your code)
Same shit as defining exception specification in interface without either providing a fallback for unexpected or forcing the exception.
Same shit as xalloc instead of new.
But in kernel this kind of shit has its own killer brand of flies…
Comment by omerm — July 3, 2003 @ 1:31 AM |
sucky is as sucky does
“Never attribute to malice that which can be adequately explained by stupidity”.
Bad programmers don’t realize that leaking even one byte of memory is the first step on the path to hell.
re System.exit() in exception handler, I’ve seen calls to exit() in *libraries*!
Same shit as using new when you mean new[], and delete when you mean delete[]
Looks like you have your share of Code That Sucks stories. Feel free to send them to me or Oleg (Goldshmidt), and we’ll put them all up somewhere, for the greater glory of the human race.
Comment by mulix — July 3, 2003 @ 1:56 AM |
Re: sucky is as sucky does
“new when you mean new[], and delete when you mean delete[]”
Excuse my ignorance but what do you mean new? with C++?
I assume you don’t refer to arrays…
Comment by Anonymous — July 3, 2003 @ 5:54 AM
Re: sucky is as sucky does
I do mean arrays and C++. Consider this snippet:
#include
class foo {
public:
foo() { std::cout << "const" << endl; }
~foo() { std::cout << "dest" << endl; }
};
int main(void)
{
class foo* p;
class foo* a;
p = new foo[3];
a = new foo[3];
cout << "calling delete[]" << endl;
delete[] a;
cout << "calling delete" << endl;
delete p;
return 0;
}
When running it, you get:
mulix@granada:~/tmp$ ./cpp
const
const
const
const
const
const
calling delete[]
dest
dest
dest
calling delete
dest
Segmentation fault
This is because memory allocated with new[] must always be freed with delete[]. Doing anything else is at best undefined behaviour, and at worst a logic error (because the destructor won’t be called on the objects).
Comment by mulix — July 3, 2003 @ 10:29 AM
Code that Sucks…
Well, I definitely have my share of this. Look, I work for a large and evil company. This means, among other things, that there are plenty of programmers involved. The law of big numbers plays its tune – there is Code that Sucks
I will send you some examples when I”ll be back to the work (next week).
Comment by omerm — July 3, 2003 @ 9:52 AM
Re: Code that Sucks…
Great, looking forward to it. Have a good weekend!
Comment by mulix — July 3, 2003 @ 10:55 AM