Thanks for the review, Darren. On Thu, Jul 31, 2008 at 11:11:48AM +0100, Darren J Moffat wrote: > Renee Danson wrote: > > Hello, > > > > I'm requesting feedback on the following bugfix: > > > > http://cr.opensolaris.org/~okie/6727748/ > > > > DJM-1 usr/src/cmd/cmd-inet/sbin/netstrategy/netstrategy.c:170 > > I don't follow why interface is declared static, what am I missing ? > It looks to me like we only return use interface (and return it) if > found_one is true and in that case we have also done a strncpy into > interface.
Jim explained this one. > DJM-2 usr/src/cmd/cmd-inet/sbin/netstrategy/netstrategy.c:204,242 > > I think these strncpy can be replaced with the safer strlcpy. After digesting the discussion between Darren, Jim, and meem on this one, I'm going to make the decision to leave it as-is. I can understand the motivation for using strlcpy, but I don't think it's strong enough to go back and change things at this point. > DJM-3 usr/src/cmd/cmd-inet/sbin/netstrategy/netstrategy.c:199 > > Existing logic, but why do we skip logical interfaces ? Zones without > exclusive IP stack often have logical interfaces. I don't expect this > to be changed for this bug fix just curious. I'll file a bug on this as Jim suggested. > DJM-4 usr/src/cmd/cmd-inet/sbin/netstrategy/netstrategy.c:235 > > The comment about only having one interface is out of date especially > given the whole reason for this fix. Yep, I'll change that. I made some other changes based on feedback from Jim, probably after you looked at this; you might want to take a look at it again, though the changes were pretty minor: * lines 84, 100: added void for the arguments to open_sockets() and close_sockets(). * line 261: initialized dhcp_running to B_FALSE, as its value might not be set in the case where get_first_interface() returns NULL (i.e. no primary interface found). * line 281: pass a pointer, rather than NULL, to get_first_interface(), so that get_first_interface() doesn't need to do NULL checks on the parameter. These changes, plus the comment changes you suggest, are now in the updated webrev. -renee _______________________________________________ networking-discuss mailing list [email protected]
