[Freeciv-Dev] Re: Pop cost patch (resending via bug system) (PR#897)

[Arien Malec <arien_malec@xxxxxxxxx>]

OK, I've got a version of the pop cost patch which incorporates the changes
suggested by this discussion. I'll send it later when I've weeded out the extra
newlines.

One question: do I also need to worry about
client/packhand.c#handle_ruleset_unit ? or does the common/packets.c fix ensure
that handle_ruleset_unit already sees a proper structure?

Arien

--- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
> On Sun, Aug 19, 2001 at 01:57:50PM -0700, Trent Piepho wrote:
> > On Sun, 19 Aug 2001, Raimar Falke wrote:
> > > > 
> > > > Are you saying the server and client know to skip the pop_cost field if
> the
> > > > capability is missing?  Or that they won't be allowed to talk at all?
> > > 
> > > Yes if 
> > > 
> > [stuff]
> > > 
> > > is changed to
> > > 
> > [other stuff]
> > 
> > Ok, I thought you meant it it would work without changes and I failed to
> see
> > how that was possible.
> 
> If the old version would work I would be scared.
> 
> > >    if (pc && has_capability("pop_cost", pc->capability))
> > >        iget_uint8(&iter, &packet->pop_cost);
> > >    else
> > >        packet->pop_cost=0; /* 0 should be a sensible default value */
> > 
> > This is better:
> > 
> >     if (has_capability("pop_cost", pc->capability))
> >         iget_uint8(&iter, &packet->pop_cost);
> >     else
> >         packet->pop_cost=(packet->flags & (1L<<F_CITIES)) ? 1 : 0;
> 
> Yes.
> 
> > There is no good reason to check pc for NULL either.  If you look, the last
> > line of the function (and all receive functions) is:
> > 
> > remove_packet_from_buffer(pc->buffer);
> > 
> > So you will get a NULL pointer reference if the function is called with
> > pc==NULL anyway.  Of course if pc==NULL that means there is a big bug
> > somewhere.  The check for NULL just confused me, because I couldn't figure
> out
> > how the code would ever get run with pc==NULL, and why it wouldn't crash if
> it
> > did.  
> 
> > Then I realized that you had just copied that line from elsewhere
> > and left in the check for no good reason.
> 
> You got me. I have also asked myself why it is so. It looks like
> something which can be removed. But I'm not sure.
> 
>       Raimar
> 
> -- 
>  email: rf13@xxxxxxxxxxxxxxxxx
>   +#if defined(__alpha__) && defined(CONFIG_PCI)
>   +       /*
>   +        * The meaning of life, the universe, and everything. Plus
>   +        * this makes the year come out right.
>   +        */
>   +       year -= 42;
>   +#endif
>     -- Patch for 1.3.2 (kernel/time.c) from Marcus Meissner
> 


__________________________________________________
Do You Yahoo!?
Make international calls for as low as $.04/minute with Yahoo! Messenger
http://phonecard.yahoo.com/