Tim Knitter wrote:
> be_activate.c:
> I agree with Joe that the "getzoneid() == GLOBAL_ZONEID" checks should 
> be in the functions if possible. Makes for cleaner code.
> 884,893,947: some variables use uppercase notation e.g. (zoneDataset 
> zoneIndex zonePath) and some don't. Please be consistent throughout.
> NIT:
> 890: zones -> zone_list seems more appropriate.


> 911: zoneDataset is being used before being set. It could be NULL here.

that should have been be_root_ds. fixed

> 954: To help with indentation and code maintenance, this line could be 
> split up and if each condition isn't true then continue. This will help 
> subsequent lines from being wrapped so much.

I don't really agree here, if we do these extra indents then all of the code 
after that will be bumping up against the 80 character limit and will get split 
up to the point none of that remaining code will be readable. I'll try to clean 
up the line wrapping here as best I can to make it more readable.

> 994: This while loop seems like a good candidate for a new function, 
> adding to code maintainability. Looks like 1096 shares this code making 
> it an even better candidate for a function. :-)

Yes that really wasn't needed there and has been removed. This while loop now 
only lives in the callback function.

> 1072: Can zhp ever be NULL? If so this line should be split up since 
> there is no error condition for when it is NULL.
> I'll review be_mount.c next and send comments in a separate mail.

Good point, I'll add a check to make sure it's not NULL.

> Thanks
> Tim
>> Evan Layton wrote:
>>> Hello All,
>>> We're down to the wire on the zone support changes to SNAP upgrade 
>>> and are looking for code review comments. We'll be taking comments up 
>>> until COB Tuesday October 7th. Your comments are as always welcome 
>>> and appreciated.
>>> Defect 3686 is the blocker bug that was submitted to cover this work 
>>> and the webrev is available at:
>>> http://cr.opensolaris.org/~equach/webrev.snap_zones/
>>> Thank you in advance for your comments and help!
>>> -evan
>>> _______________________________________________
>>> caiman-discuss mailing list
>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

zones-discuss mailing list

Reply via email to