Complete.Org: Mailing Lists: Archives: freeciv-dev: September 2000:
[Freeciv-Dev] Re: [PATCH] Names in rulesets. (PR#559)
Home

[Freeciv-Dev] Re: [PATCH] Names in rulesets. (PR#559)

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: David Pfitzner <dwp@xxxxxxxxxxxxxx>
Cc: freeciv-dev@xxxxxxxxxxx, bugs@xxxxxxxxxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: [PATCH] Names in rulesets. (PR#559)
From: Gaute B Strokkenes <gs234@xxxxxxxxx>
Date: 16 Sep 2000 06:57:12 +0200

David Pfitzner <dwp@xxxxxxxxxxxxxx> writes:

> On 12 Sep 2000, gs234@xxxxxxxxx (Gaute B. Strokkenes) wrote:
> > David Pfitzner <dwp@xxxxxxxxxxxxxx> writes:
> 
> Well, consider the suggestion made in relation to the nation name
> grammar patch, for having some fields be MAX_LEN_NAME_LONG.  By
> "hardwiring" MAX_LEN_NAME, your patch does not deal nicely with this
> sort of possibility.  We might not end up using MAX_LEN_NAME_LONG,
> but the point remains that you are making the inflexible assumption
> that all "name" strings will be MAX_LEN_NAME, which might not be
> permanently valid.

If someone makes a patch that changes the length of various buffers,
then they'd better go through the rest of the source and deal with
this sort of breakage...if not, then that someone is not doing their
job properly.  As it stands, check_name() is a nice, specialised and
local solution to a specific problem.  It wouldn't be hard to
generalise it if needed.  The only problem, I think, would be
accidential misuse.  At the same time, I don't think that people
should put too much in a name.  When you see a function like
check_name(), you might think that it also makes sure that names are
not duplicated, that they consist only of printable characters and so
forth.  It doesn't.  Maybe the name should be check_name_len() to
reflect that.  But the point remains that using a function because it
has a nice-sounding name is intrinsically a very dumb thing to do.

> I think what I really want is a version of mystrlcpy / sz_strlcpy
> (and maybe friends) which prints a log message on truncation.

That's a very good idea, actually.  I propose: check_strlen() which
does the same job as my check_name(), but with arguments for length
and error message, and a similar strlcpy() variant that calls
check_strlen().  And a sz_ macro to go with it.

I'll rewrite my patch to do this.

> (Though I know not all of the truncations involved in your patch use
> strcpy.)

s/strlcpy/strcpy

Yes.  That's ok when the thing you're looking at is destined to live
inside a fixed-size buffer in a struct somewhere, but city names
etc. are just strdup()ed (until they're used, obviously.)

-- 
Big Gaute (not to be confused with LG)
PARDON me, am I speaking ENGLISH?



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