Crash Last Night

Last night the game crashed, bringing our total crash count for 2009 up to three. The problem which caused the crash was a bit tricky to find because the actual crash was caused by memory corruption which occurred fifteen minutes before. Tracking it down and fixing it was ultimately not very difficult, but what was particularly frustrating for me in this case was that the root of the crash involved the bane of my existence: an old CODE SNIPPET.

In this case, the problem lay in the rename snippet, v1.1 written by some fellows named Voltec and belial in 1998. This particular snippet has been in the Dark Risings source for as long as I've been here (so it was likely in from the beginning), and it is what allows us imms to rename characters named "Drizzt" without forcing them to delete and recreate. However, the guys who wrote it really didn't know what they were doing (which, from my experience, has been 100% of the people who author publicly available snippets) and thought this would be a good way to rename a character:

/* grab old name, insert new name, Save the new p-file */
strcpy(old_name, victim->name);
strcpy(victim->name, new_name);

Unfortunately for every mud which has used this snippet, the authors of it did not understand that the game does not allocate entire player characters in the game on the stack as it would a local variable; characters (and therefore the names associated with them) are malloc'ed to some block of permanent memory. Using strcpy on any permanently allocated member of a char_data structure is an incredibly novice mistake; strcpy knows nothing about the size of the space in memory allocated for a character's name, so it does absolutely nothing to ensure that it doesn't either overrun the space allocated for the character's name (and therefore corrupts whatever is in the memory adjacent to it, which is what caused yesterday's crash) or release any unused but allocated memory (which would cause a memory leak).

So at best, this snippet causes memory leaks, and at worst, it overruns the boundaries of the part of memory reserved for a character's name, silently corrupts other parts of adjacent memory, and causes a mysterious crash a few minutes down the road when the game tries to allocate memory for a new string and cannot find the correct boundaries for existing strings because they got corrupted. The correct way of doing this sort of renaming routine is actually already written all over the stock ROM code, so it's not like it is even a novel concept:

free_string(victim->name);
victim->name = str_dup( newname );

ROM and ROM-derivatives have their own internal memory handling and allocation subroutines which must be used when creating and destroying strings in this malloc'ed space; that's what free_string() and str_dup() do. My guess is that the authors of the original rename snippet learned about strcpy in their high school programming class, but did not learn about the importance of understanding how the whole program works before screwing around with pieces of it. Shoddy and broken code was the result.

This is exactly the reason Dark Risings no longer incorporates any publicly available snippets. I found myself spending my Sunday morning rewriting the rename command from scratch because the snippet we have been using for it was sloppy, buggy, and caused our game to crash right in the middle of our peak hours, undoubtedly ticking off some players and disrupting the experience for everyone logged in. It took me less time to just rewrite this command myself than it took to debug the code and trace the crash back to some crumby snippet that triggered a bug a quarter of an hour before the game actually caught up with the corrupted memory and tanked.

The funniest part of all of this is that the snippet says at the top:

* also, comes complete with comments! for the budding
* coder to know whats going on */

I feel genuinely sorry for any mudder who plays on a game whose "budding coder" learned how to code from junk snippets like this. At any rate, anyone who posts an idea or suggests to me "______ should be easy to implement because there is a snippet available" will be very rudely pointed to this blog entry. Dark Risings will never use anyone else's code under my tenure as coder.