Complete.Org: Mailing Lists: Archives: freeciv-dev: September 2002:
[Freeciv-Dev] Re: [Patch] DataIO (PR#1845)
Home

[Freeciv-Dev] Re: [Patch] DataIO (PR#1845)

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: freeciv-dev@xxxxxxxxxxx
Cc: bugs@xxxxxxxxxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: [Patch] DataIO (PR#1845)
From: Davide Pagnin <nightmare@xxxxxxxxxx>
Date: Tue, 10 Sep 2002 09:27:16 -0700 (PDT)

On Tue, 2002-09-10 at 17:13, Raimar Falke wrote:
> On Sun, Jul 28, 2002 at 11:30:12PM +0200, Raimar Falke wrote:
> > 
> > The attached patch introduces common/dataio.[ch]. This replaces the
> > get and put methods of common/packets. DataIO also offers size
> > checking for put methods.
> 
> Changes:
>  - updated to current CVS
> 
>       Raimar
> 
> -- 
3 small question on the patch:

1)

/* Bitvector convenience macros. */
#define BV_DIO_PUT(buf, bv) \
  dio_put_memory(buf, (bv).vec, sizeof((bv).vec))

#define BV_DIO_GET(buf, bv) \
  dio_get_memory(buf, (bv).vec, sizeof((bv).vec))

For consistent naming, I suggest to rename BV_PUT and BV_GET macros, to
BV_DIO_PUT and BV_DIO_PUT, as the function that are replaced by them,
are colled dio_put and dio_get, instead of put and get.

2)
  if(unit_type_flag(packet->id, F_PARATROOPERS)) {

instead of that, what about:

  if (BV_ISSET(packet->flags, F_PARATROOPERS)) {
 
NOTE: If the first is better, than change also the other BV_ISSET that
happens afterwords...

3)    
  if (length > MAX_LEN_WORKLIST) {
      length = MAX_LEN_WORKLIST;
    }

Is this check, inside dio_get_worklist intended? (it seems ok to me, but
I'm not sure that it was in the previous patch)

And the big thing, I will try (but I have little time in this week) to
test this on an alpha 64 bit machine, if I'll able to, I'll send
feedback.

Ciao, Davide




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