Summary: astring interface/usage encourages memory leaks
                 Project: Freeciv
            Submitted by: jtn
            Submitted on: Sat Mar  5 19:03:26 2011
                Category: general
                Severity: 3 - Normal
                Priority: 5 - Normal
                  Status: None
             Assigned to: None
        Originator Email: 
             Open/Closed: Open
         Discussion Lock: Any
        Operating System: None
         Planned Release: 



The functions in client/text.c (and maybe other places) which return strings
make extensive use of astring internally; each function has an internal static
astring structure (wrapping a malloc'd string) which is used to accumulate
text for the duration of the function call, and then the bare string is
obtained via astr_str() and returned to the caller (who doesn't know or care
that astring is being used).

These text.c functions are erroneously described in comments as returning
pointers to static storage, which most of them haven't done since S2_0 at
least. Many callers aren't freeing their strings, so presumably our clients at
least are leaking memory (I haven't tested this).

The text.c functions return "const char *", so in fact they can't be passed
to free() (or FC_FREE()) without casting away the const (ugh). And the
const-poisoning comes from astr_str(), which also returns "const char *".

Perhaps the original intent of astring was that the lifetime of the string
storage would be tied to the astring itself and be ended with astr_free(), but
that's not how it's being used today.

To fix this I think we need to:
* make astr_str() return "char *" (and comment appropriately)
* propogate this down to those of its callers where it's appropriate
* actually free the strings in the callers-of-callers, etc (tedious)

(Unless I've missed something...)


Reply to this item at:


  Message sent via/by Gna!

Freeciv-dev mailing list

Reply via email to