Complete.Org: Mailing Lists: Archives: freeciv-dev: February 1999:
[Freeciv-Dev] Re: freeciv: static char[] problems?
Home

[Freeciv-Dev] Re: freeciv: static char[] problems?

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: Reinier Post <rp@xxxxxxxxxx>
Cc: freeciv-dev@xxxxxxxxxxx
Subject: [Freeciv-Dev] Re: freeciv: static char[] problems?
From: Greg Wooledge <ic5035%tss1crs.amgreetings.com@xxxxxxxxxxxxxxxxxxxx>
Date: Mon, 15 Feb 1999 09:24:29 -0500

On Sun, Feb 14, 1999 at 12:36:09PM +0100, Reinier Post wrote:

> > > >         char *toto;
> > > >         toto = "a message"; is wrong no memory has been allocated to 
> > > > toto.

> This is OK as long as no longer string is ever copied to 'toto'.

Attempting to write to the memory pointed to by toto will give a segfault
if this is compiled on Unix under gcc (unless -fwritable-strings or
-traditional is used).

> I am wondering about something else though: some Freeciv functions
> locally defines a string and returns it as a result.  The comment says this
> implies only one function result can be used at the same time.  I believe
> it's just broken; when the function returns, its stack is cleared, and
> as far as I remember static arrays are allocated on the stack to they
> may be overwritten from then on.  An example:
> 
>   common/shared.c:datafilename()

char *datafilename(char *filename)
{
  static char* datadir=0;
  static char  realfile[512];

[...]

  return(realfile);
}


This is OK.  Local variables marked "static" are allocated in normal
memory regions, not on the stack.  They survive after the function
returns.  Lots and lots of standard C library functions (strtok(),
asctime(), ctime(), gmtime(), and localtime() for example) are done
this way.

The drawback of this is that each time datafilename() is called, the
same object is reused; so if you write code like this you'll have a
problem:

  {
    char *fname1, *fname2;
    fname1 = datafilename (foo);
    fname2 = datafilename (bar);
    printf ("%s\n", fname1);    /* WRONG */

In this dumb example, the second call to datafilename() changes the
contents of the static array which fname1 points to.  fname1 and
fname2 both point to the same place, so a reference to fname1 will
probably not do what was intended.

I saw this problem when I performed industrial sabotage with a Spy on
a city with multiple wonders of the world in it; one of the patches I
sent a few weeks ago got rid of it in this case (by disallowing the
selection of wonders for destruction).

If there are other such problems in freeciv, I haven't yet seen them.
I won't claim that freeciv is bug-free (that would be silly); just that
I don't know of any more of this type of bug.  (For the record, the
only remaining bug I know of is the mangling of the window title when
opening a city window for cities with non-ASCII characters in their
names; and that's clearly not a memory-allocation bug.)


[Prev in Thread] Current Thread [Next in Thread]