Re: [Freeciv-Dev] Server hangs on an Alpha

[Matthew OConnor <matthew@xxxxxxxxxxxxxx>]

Nicolas Brunel [brunel@xxxxxxxxxxxxxxxxxxxx] said:

> > I changed the myrand(int size) to myrand(RANDOM_TYPE size) and
> > recompiled.  The problem was still present.  I then changed the
> > RANDOM_TYPE typedef from "unsigned int" to "unsigned long int" and the
> > now the server doesn't hang anymore.  
>
> It fixs this problem. But, it creates a cast from long int to int in lots
> of place. And casts can create problems... 

Okay I decided against that fix for the reasons you state above.  So I
did the following instead:

1) Changed line 55 of shared.h and line 314 of shared.c from:
      RANDOM_TYPE myrand(int size)
   to 
      RANDOM_TYPE myrand(RANDOM_TYPE size)

   By the way RANDOM_TYPE is typedef'ed to "unsigned int".

2) On lines 1141 to 1145 of mapgen.c I changed:
      riverbuck = -myrand(totalmass);
      mountbuck = -myrand(totalmass);
      desertbuck = -myrand(totalmass);
      forestbuck = -myrand(totalmass);
      swampbuck = -myrand(totalmass);
   to
      riverbuck = -(long int)myrand(totalmass);
      mountbuck = -(long int)myrand(totalmass);
      desertbuck = -(long int)myrand(totalmass);
      forestbuck = -(long int)myrand(totalmass);
      swampbuck = -(long int)myrand(totalmass);

   It seems to me that the above cast shouldn't be required, but w/o
   the cast it seems like compiler just skips those lines of code b/c
   they never get executed.  With the cast everything seems to work
   okay.  However I was not able to reproduce the behavior with a small
   example :/

In another email you mentioned that placing an "assert(size > 0);" in
myrand() caused everything to break (ie for generators 1 to 4).  The
reason is quite simple.  In the current source tree myrand() takes an
int, but on line 687 of mapgen.c we see the following code:

  map.seed = (myrand(MAX_UINT32) ^ time(NULL)) & (MAX_UINT32 >> 1);

MAX_UINT32 is typecast to a signed int and thus becomes -1.  That is
why the assert fails.  I poked around for a while, but that seemed to
be the only time I could get to trigger the assert(size > 0).

However that should no longer be a problem b/c I changed myrand() as I
said above.

I have attached 3 patches, they are all against FreeCiv 1.8.1.  They
are all pretty small and do exactly what I stated above.  (I realize I
should have made these against CVS and probably made them into one
patch, but it is all pretty simple).
  

> > I haven't had a chance to make
> > sure it is "working" correctly, but it no longer hangs.
>
> That's already a good point.

I played around and stepped through the source, played w/ the game and
it seems that everything is working okay.


> > (BTW both of my changes together should have counteracted the two
> >  problems you noted above).
> 
> To be really sure, we have to parse mapgen.c. In all places a cast is done
> from long int to int, we have to check the long int is no greater or
> lesser to an int. If we can wipe out those long int it will be safer.

There seems to be a lot of long ints used.  Sometimes I wonder why.
I don't know the source well enough to know if it is required in some
locations.  But there is a comment at line 1121 of mapgen.c that says
this:

  static long int tilefactor, balance, lastplaced;/* int may be only 2 byte ! */

So that makes me wonder.  Are longs just used b/c you want FreeCiv to
run on machines that have 2 byte ints?


> No time to do it tonigh,

I'd be happy to do it if you would like.

shared.h.patch
Description: Text document

shared.c.patch
Description: Text document

mapgen.c.patch
Description: Text document