[1466] in Coldmud discussion meeting

root meeting help first first in chain previous in chain previous next last

Re: diff for some things

daemon@ATHENA.MIT.EDU (Wed Nov 10 14:31:38 1999 )

From: "Bruce Mitchener, Jr." <bruce@puremagic.com>
To: <coldstuff@cold.org>
Date: Wed, 10 Nov 1999 11:13:29 -0800
Reply-To: coldstuff@cold.org

>um, bruce said i should send this here, so yell at him if i shouldn't..
>(q.v. http://web.cold.org/development.html)

Open review of this stuff is a good thing.  There aren't many people on the
patches list and as such, I doubt that much review occurs.

>it's a patch for a bug i found, when the heartbeat was turned off
>genesis would eat my cpu cycles for breakfast, running through main_loop
>like there was no tomorrow (i profiled it) -- so i poked around in the
>source and added an else to the if (heartbeat_freq != -1) there, and
>fixed it.

A few comments here:

1) The disparity between the documented interface ( heartbeat < 1 -> off )
and the implementation (heartbeat = -1 -> off) is somewhat confusing when
reading the source.  (This isn't a problem with the patch, just something
that we came across while discussing this bug on the Cold Dark).

2) Some of these fixes were already in my CVS tree (since January!).  I sent
the HEAD of that tree over to Brandon when I ceased my development, and he
put out 1.1.7.  I'm confused by the absence of some my fixes.  These fixes
were warnings fixes.

3) The number of seconds that is assigned as the default (2 in this case) is
an important number.  This is what defines how long something will have to
wait that does a pause() or a refresh() that ends up pausing in the absence
of any network traffic.  It'd be nice to see this part of the code using a
real timeval and only pausing for 0.5 seconds or so, but that is a good deal
more work.

>also included are some patches for compile warnings i get.
>being that i have yet to become one with diff, i might have done
>something stupid in making the patch. deal.

Looked fine to me.

 - Bruce