[CF-Devel] Bugs in 1.3 with fixes

[Kurt Fitzner]

Hi all,

I've been running my own server for a few weeks now.  Started as just a game
for me and my son to play together, but now I'm interested in getting some
role-play going on it.  When I allowed public logons, I soon discovered that my
Linux firewall box was too light on memory to run the server properly, so I
decided to move the server to an HPUX box.  This proved interesting.  There
are a few bugs in the server that don't matter so much in Linux, though I'm
sure they are showing up as intermitant crashes.  I'll outline what I've found
below (all symptoms are as they appeared on my HP 9000 model 700 B132L PA-Risc
1.1 with HPUX 11.0)

1) SockList_ReadPacket(), socket/lowlevel.c
After an addme command from the client, read()s in SockList_ReadPacket start
returning a zero and errno is set to ENOENT (file not found).  I noticed
somewhere a comment about duplicating a file descriptor during an addme.  I
didn't really look into this issue too well, since I found that if I ignore
the errors and don't close the socket that it works ok.  I started out by
ignoring zero return values from read()s only when errno was ENOENT, but I
found that in one specific case, when a client connects and disconnects before
logging in, that subsequent logins then started causing zero return of read()s
immediately on connection (not after the addme), and errno was set to EPIPE. 
Now I ignore all zero return values from read()s where the errno is not
ECONNRESET (connected reset by peer).  I occasionally get some oddness, but
very rarely.  I think the whole socket issue around addme needs looking at.  I
have something that 'works for me', but I'm dead sure it's not portable, and
what's there now is more than likely subtly broken.

2) new_save_map(), common/map.c
There is logic at the beginning of the function to determine if the map is a
temp map and not compress it if it is.  The logic is:

   if (m->compressed && (m->unique || flag))
     /* popen() and compress it */
   else
     /* fopen() and don't compress it */

There is logic at the end of the function to determine, again, if we have just
saved a temp map, and if so fclose() the file, if not, pclose().  This is that
logic:

    if (m->compressed && !flag)
        pclose(fp);
    else
        fclose(fp);

Seems as if the logic is reversed at the bottom.  The end result, is that temp
maps never get fclose()d.  On Linux this seems to not really matter, because
even open files get completely flushed periodically to disk.  On HPUX,
however, the last <=4k is not flushed unless an fflush() or fclose() is
called.  Even on Linux, though, you'll end up with gazillions of open files,
and this will leak memory and cause other problems.  I changed the ending
logic to match the opening, and it works well now.

3) get_empty_treasure(), common/treasure.c

Here we malloc() a new treasure struct and initialize it.  Unfortunately, the
code doesn't initialize the change_arch struct.  On my HP this crashed the
server almost any time new treasure was created, since HPUX pointers are
virtual.  ix86 are virtual pointers too, but Intel goes through more pain to
make them look real, so if you have a garbage pointer in change_arch.name on a
Linux box, you'll happily end up with an object with a funny name.  On
the other hand, you might end up pointing to a point of memory with a megabyte
of non-null values and crash the game.  Or maybe Linux zeros out
malloc()ed memory.  Does someone know, for interest's sake? Anyway, Simply
initializing the change_arch struct's three pointers to NULL here might fix
nigling little crash bugs on Linux.  It makes it runnable on my HPUX.

That's all for now. I'll post more as I find them.  I apologize for the
lengthy message.  You all will be more familliar with what symptoms are
showing up on Linux boxes after lengthy runs than I am, so I felt a little
discussion was better than just a patch.

I'm noticing that some (notably outdoor themed) random maps are showing up
populated with doors and monsters, but no background.  Don't know if I'm
missing maps or if there's a bug there.  If anyone knows and can save me time
chasing that one, I'd appreciate knowing.

Thanks.

                Kurt Fitzner (a.k.a. reven at crossfire.excelcia.org)
-- 
    _/_/_/ _/ _/    _/_/_/ _/_/_/ _/     _/_/_/ _/_/_/ _/_/  kfitzner@
   _/      _/_/    _/     _/     _/     _/       _/   _/ _/  excelcia.org
  _/_/      _/    _/     _/_/   _/     _/       _/   _/_/_/
 _/        _/_/  _/     _/     _/     _/       _/   _/   _/  E-Dad
_/_/_/    _/ _/ _/_/_/ _/_/_/ _/_/_/ _/_/_/ _/_/_/ _/    _/  PGPid 0xF621EDAD