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

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

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