[quagga-dev 1091] Re: point-to-point patch

Greg Troxel gdt at ir.bbn.com
Wed Apr 28 16:44:41 BST 2004


> I'm sympathetic to not wanting to change the wire protocol, since I guess
> that would allow independent upgrading of various daemons.  But there
> is an easy fix to that problem:
> in lib/zclient.c:zebra_interface_address_add_read(), instead of reading
> a flag byte, one can simply test the address after reading it in,
> and, if it's all zeroes, just replace it with a NULL pointer.  In fact,
> that's the way I coded it up the first time, but then I got nervous
> about whether INADDR_ANY could ever be a valid destination address.
> So I don't think wire protocol compatibility bears on the larger question
> of whether to use NULL pointers or zero destination addresses.

That's a good point, and my real issue was with the format change, so
that sounds reasonable.

> When I developed this patch, I based it on the behavior of the existing
> code.  Currently, the software either checks for a NULL destination pointer
> or completely ignores the question of whether a valid destination address
> is available and goes ahead and uses it without checking.  So I decided
> to make the software consistent and always check for a NULL pointer before
> using the destination address.

That sounds good.

> I may be wrong, but I do not believe that there is any existing code that
> checks for a zero (INADDR_ANY) destination address.  So if you guys would
> like to shift to that paradigm, it will actually be a larger patch.
> Here's what the new patch would look like:
>    1. Whereever struct connected's are created, code will have to be added
>       to create an INADDR_ANY destination address if none was supplied.
>    2. Whereever the current code tests for a NULL destination address
>       (for example, zebra/interface.c:connected_dump_vty()), that will
>       have to be replaced by a test for INADDR_ANY or IN6ADDR_ANY, depending
>       on the prefix family.
>    3. Whereever the current code was using the destination address without
>       checking, it will have to be patched to test for INADDR_ANY or
>       IN6ADDR_ANY, depending on the context or prefix family.
> My current patch only includes the 3rd part, plus the actual coding
> of part 3 is simpler because you don't have to worry about the prefix family.
> 
> So that's why I think NULL pointers are the right way to go: I think
> it's a smaller patch that is more consistent with the existing behavior.

I don't have an issue with NULL pointers; they are semantically
sensible.

I would like to see the rules about null pointer semantics, and use of
structures with it in comments, and probably mentioned in the
ChangeLog as well.

> But you guys are in charge, so I defer to your judgement.

Thanks,  but there are four of us, and probably six opinions...




More information about the Quagga-dev mailing list