* * * * *
Rabid howler monkeys on crack wrote this code
Okay, yes, there are issues with the code to The Protocol Stack From Hell™
[1]. There's the vowel impaired names—oh, sorry, the vwl_imprd_nms, the usual
ignoring return codes and tons of global variables—sorry, glblvrbls littering
the code. And there's stuff like:
> foo_t *p = NULL;
>
> /* lots of code not touching p at all */
>
> if (p) {
> /* lots of code that will never be executed because */
> /* p is always, *always* NULL at this point */
> } else {
> /* this code will always *always* be executed */
> /* p is never touched otherwise */
> }
>
> /* p is still never used */
>
Yes. At one point p was probaby used, then a code change sometime during the
Clinton Administration [2] (late first term most likely) removed the need for
p but later code still checked it, so in order to keep the code from crashing
(during the last year of the Clinton Administration, most likely) the
“easiest fix that would work with minimal code changes because we want to
avoid a five day regression test” is to just NULL out the variable where
declared and call it a day.
Odder yet is the code that generates a string, checks to see if the generated
string ends with two newline characters and then adds one or two newline
characters if required (and yes, it checks for the first newline character,
then the second) and further down in the code, it checks to see if the line
has two newline characters and carefully removes them, one at a time.
Yes. The code adds two characters, only to remove them later on.
Again, I can see the requirements late during the Reagan Administration [3]
bumping up against the requirements during the early Bush 43 Adminstration
[4] and again, the easiest way to handle this is a local change that distrubs
as little code as possible.
Although, there is one bit that does smack of rabid howler monkeys on crack
taking a pass at the code, which I briefly mention in passing [5]. It's
basically the Poster Child™ for why certain C programmers should be taken out
back behind the shed and disembowled with a grapefruit spoon [6].
Back then, I was tasked with modifying some code to log the Protocol Stack
From Hell™ errors via syslog(), and all I had to work with was a C source
file:
> /* tons o' broilerplate text whereby we pledge our first born to feed the
> * lawyers of The Protocol Stack From Hell™ */
>
> void Stpd_Init_Fnctn(void)
> {
> MYSTERIOUS_ENTRY_CODE_WE_CANT_TOUCH();
> /* modifications here */
> MYSTERIOUS_EXIT_CODE_WE_CANT_TOUCH();
> }
>
> void Stpd_Lrm_Fnctn(lrm_t *ptr,int wat)
> {
> MYSTERIOUS_ENTRY_CODE_WE_CANT_TOUCH();
> /* modifications here */
> MYSTERIOUS_EXIT_CODE_WE_CANT_TOUCH();
> }
>
> /* Muahahahahahahahahahahahaha! */
> /* [S/X thunder ] */
>
(No, seriously, each function starts and ends with
MYSTERIOUS_something_CODE_WE_CANT_TOUCH()) and an object file, which the C
code is linked against to produce the final program.
Okay, nothing that out of the ordinary. Only we weren't getting the proper
error messages from the lrm_t … thingy … we were given. Some back and forth
with The Protocol Stack From Hell™ Technical Support® and we had the final
solution, and if you can read C code, prepared to be horrified:
> void Stpd_Lrm_Fnctn(lrm_t *prt,int wat)
> {
> char *msg;
>
> MYSTERIOUS_ENTRY_CODE_WE_CANT_TOUCH();
>
> msg = (char *)prt + sizeof(lrm_t);
> syslog(LOG_WTF,"error: %s",msg);
>
> MYSTERIOUS_EXIT_CODE_WE_CANT_TOUCH();
> }
>
For those not fluent in C, let me translate: “you will receive a block of
memory called prt, which has a particular layout we laughingly call lrm_t.
Ignore the data there, but instead, what you actually want lies just past the
block of memory you received, into an area that Standard C calls “undefined
behavior.” Abandon all hope ye who program here. And have a nice day.”
Then, I was horrified. Now, I get to see the code from “the other side” and
“horrified” does not describe my reaction. “Running away, screaming in sheer
madness of having peered deep into the Abyss” would be a bit closer, but
still misses the mark. It goes something like this.
> typedef struct {
> /* data data data */ /* [1] */
> } lrm_t;
>
> typedef struct {
> /* a mass of data */
> arbitrary_size_t reserved[6]; /* [2] */
> } msg_t;
>
> void rnd_fnct(int wat)
> {
> /* don't worry, these are big enough */
> char inbffr[256],tmpbffr[256],msgbffr[256]; /* [3] */
> msg_t *msg;
> lrm_t lrm,*plrm;
>
> /* a bunch of code to receive an SS7 message and determine that the */
> /* contents need to be logged, as it indicates an error */
>
> msg = (msg_t *)inbffr;
> frmt_rnd_msg(msgbffr,msg); /* we know how big the resulting buffer is */
>
> /* okay, now for the real horror show */
> memcpy(&lrm,msg->reserved,sizeof(lrm_t)); /* [4] */
>
> /* No! Don't go into the basement! */
> memcpy(tmpbffr,&lrm,sizeof(lrm_t)); /* [5] */
>
> /* The call originated from inside the house! */
> strcpy(&tmpbffr[sizeof(lrm_t)],msgbffr); /* [6] */
>
> /* Aieeeeeeeeeeeeeeeeeeeeeeeee! stabbity stab stab */
> Stpd_Lrm_Fnctn((lrm_t *)tmpbffr,wat); /* [7] */
> }
>
And now for the play-by-play commentary on this horror show:
1. This describes the layout of the memory block we're given in
Stpd_Lrm_Fnctn(). It's not terribly big as structured memory goes, maybe
around 60 or 70 bytes, but it primarily contains useless information as
I found out.
2. It's a slightly larger block of memory, but notice the last field,
reserved. A comment in the code says that this area is for “internal use
only” and is around 24 bytes in size.
2. Keep in mind—this field is only 24 bytes in size. The size of the block
of memory we're given in Stpd_Lrm_Fnctn() is around 60 or 70, which is
larger than 24. 24 is smaller than 60 and 70. This is important.
3. “256 bytes should be enough for anyone, right?”
3. This, on a system with gigabytes of memory.
4. This copies data out of the reserved field into a block of memory of
type lrm_t. Refer back to note 2. Notice how we want to copy 60 or 70
bytes of information, but the field we're copying from is only 24 bytes.
This, my friends, is known as “undefined behavior” in C.
4. Only, this is air quotation marks okay air quotation marks because msg_t
is technically the air quotation marks header air quotation marks of a
larger message and thus, we can air quotation marks safely air quotation
marks copy memory past the end of the header.
4. My thinking here—the error codes originally fit into the space set aside
by reserved but grew over time, found out, but too much code relied on
this situation, so they're stuck with it.
4. Or, you know, rabid howler monkeys on crack.
5. I just hope that lrm_t doesn't ever exceed 256 bytes in size.
6. I'm serious. I'm not making this up. The code actually uses strcpy().
6. strcpy() is bad because there is no checking to see if you have overrun
the space set aside to receive the copied string.
6. The use of this function should cause modern C compilers to bitch
mightily and stop compilation right then and there, and send the
programmer to jail, do not pass Go, do not collect $200.00. Especially
if the programmers are rabid howler monkeys on crack.
6. This. Is. An. Ex. Function!
7. And now we call our function. Hope the error message, plus the lrm_t,
didn't exceed 256 bytes.
Rabid howler monkeys on crack.
I'm serious.
There aren't enough facepalms to do this code justice.
[1]
gopher://gopher.conman.org/0Phlog:2012/01/30.2
[2]
http://en.wikipedia.org/wiki/Presidency_of_Bill_Clinton
[3]
http://en.wikipedia.org/wiki/Presidency_of_Ronald_Reagan
[4]
http://en.wikipedia.org/wiki/Presidency_of_George_W._Bush
[5]
gopher://gopher.conman.org/0Phlog:2010/11/30.1
[6]
http://en.wikipedia.org/wiki/Grapefruit_spoon
Email author at
[email protected]
