Hi James and all, > 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.
Even if you don't consider your work a review, this is a lot already. :-) > 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. Glad to hear. Thanks. > 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.) I would very much appreciate if someone else would review the changes. Thank you for your comments on the code. > Some notes on the code itself: > > - Please remove references to the CR number from the comments in the > code. Done. > - is_our_address() looks like a boolean_t function, not an int > function. Yes, sorry. > - 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. Changed. > - 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. Changed. > - Please remove the 'iflist traversal taken ...' comment; it doesn't > explain the code. Done. I've updated the webrev at http://cr.opensolaris.org/~nigoroll/6667021_dhcpd_purge_own_offer/ Thank you, Nils _______________________________________________ networking-discuss mailing list [email protected]
