[Freeciv-Dev] Re: [PATCH] User-configurable default tileset

[Jason Short <vze2zq63@xxxxxxxxxxx>]

Baumans wrote:
> Here's the next version.  It uses fc_malloc to initialize the strings (with
> a length of MAX_LEN_OPTION_STR), and then, if there are no commandline
> arguments specifying otherwise, it copies what it loads from the file to the
> original variables.  This version also includes gui code for the xaw client
> (though it's untested, because I can't get the xaw client to work. The code
> in civclient.c is relatively messy, but I can't think of a better way,
> except for a modified version of my original:  we use mystrdup for those
> variables when we load them, or when they're modified ("free"ing
> appropriately), and we use mystrlcpy to copy these into the variables that
> are actually used in the game.
The added code in civclient does seem a bit much.  What about declaring 
the strings as arrays instead of pointers, i.e.
     char *default_tileset = NULL;

     ...

     default_tileset = fc_malloc(MAX_LEN_OPTION_STR);

would become

     char default_tileset[MAX_LEN_OPTION_STR] = "";

you could then drop the "***_modified" check and just assume that if the 
string is non-empty then it has been modified.  Everything else should 
continue to work with this change.
(Personally, I didn't see a problem with the original system of 
re-allocating every time the string was changed [this is the client, not 
the server after all], but I know Raimar at least doesn't like such 
tactics.)

Also, the XAW client works acceptably under the patch now (although I 
don't know how to make the text fields wider, which would be nice).

P.S.  If you increase the numbering of your patches (i.e. 4->5) it makes 
things easier to track.
P.P.S.  You should submit this to the bug-tracking system soon.  Just 
send the next patch to bugs@xxxxxxxxxxxxxxxxxxx instead of freeciv-dev.
jason