Complete.Org: Mailing Lists: Archives: freeciv-dev: January 2002:
[Freeciv-Dev] Re: Question about attributes
Home

[Freeciv-Dev] Re: Question about attributes

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: Mike Kaufman <mkaufman@xxxxxxxxxxxxxx>
Cc: jdorje@xxxxxxxxxxxxxxxxxxxxx, freeciv-dev <freeciv-dev@xxxxxxxxxxx>
Subject: [Freeciv-Dev] Re: Question about attributes
From: Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>
Date: Thu, 10 Jan 2002 21:10:09 +0100
Reply-to: rf13@xxxxxxxxxxxxxxxxxxxxxx

On Thu, Jan 10, 2002 at 01:08:58PM -0600, Mike Kaufman wrote:
> On Thu, Jan 10, 2002 at 12:56:31PM +0100, Raimar Falke wrote:
> > On Thu, Jan 10, 2002 at 06:37:38AM -0500, Jason Short wrote:
> > > Raimar Falke wrote:
> > > 
> > > > On Thu, Jan 10, 2002 at 05:53:23AM -0500, Jason Short wrote:
> > > >>Raimar Falke wrote:
> > > >>
> > > >>>You see the attributes aren't really used. I agree that a seperate
> > > >>>attribute_length is the way to go. Please send a patch.
> > > >>>
> > > >>What about just malloc'ing the buffer in attribute_get?  This saves the 
> > > >>extra function call (which will generally always be necessary), at the 
> > > >>small cost of having to free the buffer later.
> > > > 
> > > > This all depends on the usage patterns. So far we have not enough
> > > > users to make a final decision. The agents I have coded so far doesn't
> > > > need these attribute_length call at all because they know the size. If
> > > > more users like the tile markers are created we may have to rethink
> > > > this. Also note that the tile markers in at least one case needs a
> > > > slightly larger buffer.
> > > 
> > > Interesting.  Keep in mind that this data is unverified; if you connect 
> > > to a server in place of some former player, and that player has uploaded 
> > > invalid attributes, your client should not die.  So it is important that 
> > > error-checking be done in all cases.
> > 
> > I agree. The error-checking which is in place is via asserts.
> 
> this is not good enough. This will cause the client to die via this
> assert: assert(max_data_length >= length); if the previous player had
> larger sized attributes than I (and these are not necessarily "invalid"
> attributes, after all, they're client data).
> 
> Or you're forcing users not to screw around with "blessed" attributes.

For testing (and I consider the attributes still in the testing phase)
defensive programming is a good choice IMHO. So you can catch user and
code errors. If you are too relaxed you won't catch the code errors
because there are masked as user errors.

About the problem that you take over another user: if the first client
uses other organization/size of an attribute (one ATTR_* enum) that
the second client there is nothing you can do to convert/understand or
even catch the problem (you will only notice something if the size
changes). Example: old version (used by the first client):

struct {
  char x, y;
}

New version (used by the second client) changes the attribute format
to:

struct {
  int some_pos_index;
}

The size won't change and the second user won't notice it.

We may ask the users of the attributes to add a version field to catch
this but I don't see a problem you can enforce this in the generic
attribute handling code.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Using only the operating-system that came with your computer is just
  like only playing the demo-disc that came with your CD-player."


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