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

[Raimar Falke <rf13@xxxxxxxxxxxxxxxxx>]

On Tue, Sep 10, 2002 at 06:25:21PM +0200, Davide Pagnin wrote:
> 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.

I would put them under the names of dio_{put,get}_bitvector in
dataio.h BUT this isn't possible since they require macro magic.

So I would suggest moving them to dataio.h under the name
DIO_{PUT,GET}_BITVECTOR.

> 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...

I think that there is no real better-one. I agree that it should be
consistent.

> 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)

packets.c was changes in the mean time.

> 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.

Yes please do.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  This customer comes into the computer store. "I'm looking for a mystery
  Adventure Game with lots of graphics. You know, something realy
  challenging". "Well," replied the clerk, "have you tried Windows 98 ?"