Nils Goroll writes:
> Peter, Victor, James and all,
>
> after the discussion at the end of last month, do you agree that the fix
> should
> be integrated or does anyone have objections?
>
> http://cr.opensolaris.org/~nigoroll/6667021_dhcpd_purge_own_offer/
I've been busy with other things, so I haven't had time to look at
this. You may need to get someone else to review it.
The design seems reasonable to me; it looks like we just avoid purging
offers if the server address is one of our other addresses. The
reasoning behind this seems sound.
Unfortunately, I don't know enough about the code you're changing to
know if there might be other issues related to this change. (I think
meem or perhaps Dave Miner would know better.)
Some notes on the code itself:
- Please remove references to the CR number from the comments in the
code. We generally don't embed CR numbers in comments for things
that are *fixed*, because (a) if it's really fixed, then the CR
isn't that interesting anymore, (b) if anyone needs the history,
that's what 'hg history' is for, and (c) the CR number itself
doesn't explain the code.
- is_our_address() looks like a boolean_t function, not an int
function.
- It shouldn't be necessary to pass 'serverid' by reference to that
function; it's just a 4-byte integer value. Either passing it as
a structure or passing .s_addr (as in_addr_t) would be preferable.
- I don't think that the use of ifp_mtx does much here; the read of
ifp->addr.s_addr is an atomic action anyway. It's a nit, but I'd
remove the use of that lock.
- Please remove the 'iflist traversal taken ...' comment; it doesn't
explain the code.
--
James Carlson, Solaris Networking <[EMAIL PROTECTED]>
Sun Microsystems / 35 Network Drive 71.232W Vox +1 781 442 2084
MS UBUR02-212 / Burlington MA 01803-2757 42.496N Fax +1 781 442 1677
_______________________________________________
networking-discuss mailing list
[email protected]
