Update: Check out my wiki page for the latest download of WinROTT.
While moving the WinROTT file w_wad.c to w_wad.cpp, I ran into the most interesting bug. It was very difficult to catch. The problem was only apparent when the game was compiled in Release mode. After inserting a lot of logging statements, I was able to trace the problem down to this routine.
int W_CheckNumForName (char *name)
{
char name8[9];
int v1,v2;
lumpinfo_t *lump_p;
lumpinfo_t *endlump;
// make the name into two integers for easy compares
strncpy (name8,name,8);
name8[9] = 0; // in case the name was a fill 8 chars
strupr (name8); // case insensitive
v1 = *(int *)name8;
v2 = *(int *)&name8[4];
// scan backwards so patch lump files take precedence
lump_p = lumpinfo;
endlump = lumpinfo + numlumps;
while (lump_p != endlump)
{
if ( *(int *)lump_p->name == v1 && *(int *)&lump_p->name[4] == v2)
return lump_p - lumpinfo;
lump_p++;
}
return -1;
}
W_CheckNumForName(char *name) looks up the lump's index by looping through the collection, comparing names. The interesting point is that the lump name is shortened to 8 characters and then converted to two four byte integers. The integers for the lump are compared to the integers for the desired name.
This routine may not work correctly when the name we are searching for is less than 8 characters long. When *name is less than 8 characters, the technique of converting to two four byte integers may fail. Why? Because the characters after strlen(name), up to character 8, are undefined. Any values may be in memory for these bytes. So the second inteteger, v2 may vary in value. Now along comes strncpy_s, the "safe" copy routine. It takes it upon itself to fill every unused byte with 0xFE. This is a good thing, unless you happen to be playing a game saved using the old method.
So I began to think, what would happen if we simply used strcmp to compare the string names? I think the original authors were concerned about performance, which lead them to use the integer comparison technique, but modern processors are so much faster, using strcmp is a lot more reliable and the performance hit shouldn't be too great. So here is my fix to the lookup problem. I still have the problem of depending upon pointer arithmetic to return the correct index. This can be easily fixed using a counter.
int W_CheckNumForName (char *name)
{
char name8[9];
lumpinfo_t *lump_p;
lumpinfo_t *endlump;
// make the name into 8 bytes for backward compatibility
int l = (strlen(name) > 8)? 8:strlen(name);
strncpy_s(name8, 9, name, l);
_strupr_s(name8, 9); // case insensitive
// loop through the list
lump_p = lumpinfo;
endlump = lumpinfo + numlumps;
while (lump_p != endlump)
{
if (!strcmp(name8, lump_p->name))
return lump_p - lumpinfo;
lump_p++;
}
return -1;
}