Saturday, October 3, 2015
VerySillyMUD: Remaining compilation errors

This post is part of my "VerySillyMUD" series, chronicling an attempt to refactor an old MUD into working condition[1].

In this edition, we'll try to fix all the remaining compilation errors. The next error we have is in handler.c:

As before, we'll fix this by converting it into a comment. The code clearly isn't used, but this is an unfamiliar codebase so I don't want to throw code away (yet). Easy enough. Next error:

Now this is truly frightening. Someone is calling malloc with the wrong number of arguments; this is almost certainly the source of some kind of nefarious memory bug. What's going on here? A glance at the source code enlightens us:

The parallelism in the code here now makes this clear once we can see the associated realloc() call, which takes a pointer to the old allocation (script_data[top_of_scripts].script). This pointer is not needed for the initial malloc call, so we can remove it. Next up we have:

Ok, that's a straightforward error, but how do we fix it? Let's look at this function:

The error occurs at the first return statement. This function appears to reference special procedures; as I recall, certain areas or creatures ("mobs") in the MUD had unusual behaviors or triggers. Looking at the rest of the body of this function, it seems to be looking for special procedures and firing them, returning a 1 if one was found. Since the first return statement doesn't seem to fire any, it seems that returning 0 there is probably the correct return value. Next error:

Here the code is using a deprecated header file (malloc.h) and we can just include stdlib.h instead. Fixing that let the compilation of utility.c get further:

Ah, our good friend "#ifdef 0" again. Let's turn you into a comment instead. Next:

Hmm, we'll have to take a look at this shop_keeper function, which presumably operates the many places in the MUD where your character could buy items.

Ok, what seems to be happening here is that the function returns TRUE when the shopkeeper has performed some special behavior, and FALSE otherwise. It seems like the three return statements causing the errors are the ones up at the top of the function doing early exits from the function; it seems reasonable to guess that we should return FALSE in these cases, but we really don't have any way to tell without a much closer understanding of the codebase, which we're not going to attempt to get yet. Next error:

Another missing return value; we'll guess we should supply another zero here for this early-exit case.

More missing return values! This time, it turns out all the return statements in the DamageMessages() function don't have return values; this leads me to suspect the return type of this function is wrong. A quick grep confirms that none of the callers capture or inspect a return value, so we'll just change the return type to void. Next:

As with the shop_keeper() function we saw above, the missing return value is on an early exit from the function, which manages characters interacting with the bank and returns TRUE if such an interaction occurs. The early exit happens if IS_NPC() is true for the character. An NPC is a non-player character, i.e. one played by the program and not by a human; presumably NPCs can't interact with the bank, so it seems returning FALSE in this case would be right. Next:

We saw log_msg() in our previous post. It's defined this way:

Note the commentary left by a developer on this one-liner, which is near the top of its file and hence looked like a function prototype. At any rate, we see that log_msg() is really shorthand for calling a different logging function, log_sev(), with a priority of 1. It seems like our error in board.c was either meant to call log_sev() directly or just accidentally added an argument:

Since the other error message here just invokes log_msg() for what would seem to be an equivalently serious error, we'll change the problematic invocation to match. Next:

More missing return values. We'll default them to returning FALSE. Next:

If we supply one more FALSE then the whole thing compiles into an executable called dmserver. Let's try to run it!

Well, not so hot, but we'll leave fixing it to the next episode.

[1] SillyMUD was a derivative of DikuMUD, which was originally created by Sebastian Hammer, Michael Seifert, Hans Henrik Stærfeldt, Tom Madsen, and Katja Nyboe.

VerySillyMUD: Compilation

This post is part of my "VerySillyMUD" series, chronicling an attempt to refactor an old MUD into working condition[1].

The first goal is just to get the thing to compile. The source code is early 1990s C, and I have this C compiler on my computer at home:

The compiler doesn't quite like the source code here; it generates 21 warnings and 20 errors on the first file when we run make! I haven't included the full output here for readability's sake, but you can peruse the full gist if you like. Our goal here is to get compilation to succeed while making only minor edits that we think are safe and do not change intended functionality.

The first error is here:

The error in question points out a conflict between the prototype included here and the system-level include file string.h. A quick grep of the source code shows that strdup() is not implemented in the source code here, so this should just be a matter of removing the prototypes where we find them in the source code and instead just including them via the system level library, as seen in this commit. The next error/warning is:

If we look at the source code in question, we see:

This could either be a mistake in the function definition (i.e. it should return void instead of int) or this is an oversight and the function ought to be returning some kind of status code as an integer. A quick grep shows this is actually only called from one place (elsewhere in comm.c) where the return value is not captured or inspected. Therefore we can take care of this in the short term by correcting the function declaration, as captured in this commit. What's next?

Ok, from looking at the error message, the compiler seems to be confusing a locally-defined log() function that outputs some kind of logging message with the one from the math.h library for computing algorithms. The easiest way to fix this is to rename the local function for clarity to something like log_msg(), as captured in this commit. Ah, rats, now we see that this function didn't actually have a prototype anywhere, or at least not in enough places:

Adding the prototype to the protos.h include file seems to fix that up. Now, I just noticed at this point the compilation of comm.c still generates 25 warnings but only 2 errors! It turns out the very first fix we did for strdup cleaned most of them up. As I am running out of time for this session, let's press on to try to clean up the last two errors and at least get one file compiling. I'm generally a proponent of the "treat compiler warnings as errors" rule of thumb, but I think we're not quite ready to add that rule. I'd really like to get to the point where we can get some unit tests in place before doing too many more modifications. The first remaining error is:

Hmm, this is a new one for me. The bind() function connects a socket to a particular address so that we can start accepting connections from clients there. The system library's bind() on Mac OS X only takes 3 arguments, as does Linux's. A look at the source code shows there's an extra 0 argument being passed. Maybe this was for an older version of bind? According to running man 2 bind on my Mac OS X computer, in the HISTORY section it says "The bind() function call appeared in 4.2BSD." It turns out if we actually look at the man page for bind() from 4.2BSD it also only takes 3 arguments! What the heck? Without really knowing what the compilation target was for the codebase, it's hard to say what to do here. I'm going to assume one of the compilation targets had a bind() call that maybe took some optional flags as a fourth argument, but the source code didn't opt into any of them (hence the zero). According to this, extra arguments to functions were probably safely ignored, so I think we can just get rid of the extra argument here and see if that works. It'll certainly clean up the error at least. Ok, the last error, then is:

I think this dates me as a C programmer, because I thought that was still a valid way to #ifdef-out a section of code. I'll just comment the code out instead. Update: when I mentioned this on Twitter, Steve Vinoski pointed out:

This was interesting because Steve was right in that I had forgotten the idiom I was used to (#if 0). But it was also interesting that this code, which apparently worked at one point, used this incorrect idiom (#ifdef 0). A little experiment shows that this is not valid in C89 ("ANSI C"):

If I attempt to compile this using C89 compatibility, I get:

So this was written for a compiler with non-ANSI C support. It doesn't even appear to be valid K&R C, according to the list of gcc incompatibilities between it and ANSI C (the version of gcc I have installed is based on the clang compiler, which doesn't support the -traditional flag, and I'm not motivated to go install an older version to try it out). There are hints in the source code that this was targeted towards Sun computers; perhaps their early-1990s-vintage proprietary system C compilers accepted this syntax. Anyway, it's an easy enough fix, so let's not dwell further on it.

(Partial) success! At this point, the make run actually builds 10 object files (including comm.o) before erroring out (full gist). Next time out we'll continue fixing compilation errors in the hopes of getting at least a clean compilation run.

[1] SillyMUD was a derivative of DikuMUD, which was originally created by Sebastian Hammer, Michael Seifert, Hans Henrik Stærfeldt, Tom Madsen, and Katja Nyboe.

VerySillyMUD: An Exercise In Refactoring: Intro

When I was in college, I played a MUD called SillyMUD, which was in turn a derivative of DikuMud, which was written by Sebastian Hammer, Michael Seifert, Hans Henrik Stærfeldt, Tom Madsen, and Katja Nyboe. I happened to mention it to one of my kids, who expressed interest in seeing/playing it.

I found an old source code archive of it, and it turns out it was distributed under an open source-style license. Unfortunately, it doesn't even compile as-is, and as I recall there were several scalability and memory bugs present in the codebase even when I played it.

Therefore, I decided that I would undertake gradually refactoring the thing into shape and perhaps modernizing some of the technology choices in the process, while chronicling it here on the blog to capture it for educational purposes as an example.

The posts in this series so far are:

  1. Compilation
  2. Remaining Compilation Errors

More to come!