[Freeciv-Dev] Re: (PR#8720) Dynamtext

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=8720 >

Raimar Falke wrote:
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=8720 >
> 
> On Fri, May 21, 2004 at 11:58:26PM -0700, Jason Short wrote:
> 
> Here is a patch. It is a step toward the goal. If this is applied I
> will make another one to change text.c and change the cases in
> common/*

OK, except...

> +/*
> + * An formatted string in vadd() has to fit into VADD_TMP_SIZE. One
> + * buffer of VADD_TMP_SIZE size will be allocated for the entire
> + * program.
> + */
> +#define VADD_TMP_SIZE        (1024)

<MARK>

> @@ -80,6 +90,9 @@
>    n1 = (3*(astr->n_alloc+10)) / 2;
>    astr->n_alloc = (n > n1) ? n : n1;
>    astr->str = (char *)fc_realloc(astr->str, astr->n_alloc);
> +  if (was_null) {
> +    astr_clear(astr);
> +  }
>  }

was_null shouldn't just be set if the original string was NULL.  If you 
reset a string to length 0 and then increase the length again it should 
also work.  So instead of

   bool was_null = astr->str == NULL;

it should be

   bool was_null = (astr->n == 0);

also don't call astring_clear if n == 0.  Instead it will be called next 
when the size is increased.

This probably isn't a concern for your code, which only increases the 
string size.  But it's better to cover all cases.

> +/****************************************************************************
> +  Add the text to the string.
> +****************************************************************************/
> +static void vadd(struct astring *astr, const char *format, va_list ap)
> +{
> +  size_t new_len;
> +  static char buf[VADD_TMP_SIZE];

See <MARK>.  WHy is this buffer static?  This seems totally unnecessary.

jason