Sue, Thanks for the review... comments inline.
-ethan Sue Sohn wrote: > Below are my comments on the incremental: > > General: In be_list.c, be_snapshot, and be_rename, returns that used to say > "be_errno_t", now say "non-zero". But in be_mount.c, they still say > be_errno_t. > Should be consistent. We'll make it consistent. > > be_list > 182 I think it's easier for translation if it says "BE does not exist %s\n" Okay. > > be_mount > 243 it's->its okay. > 296 Could altroot potentially be NULL here? I suppose it could be since its passed in and we're not validating it. We'll check it upon entry. > 488 function in error message is wrong okay. > 546,762 filesystems's->filesystem's okay. > > be_snapshot > 69-74 indentation 69-72 look fine. I'll fix 74. > 274 I am not sure that it works for gettext if you put a constant in the > string. > Please double check. If it does work, then there a several "legacy"s to > change > in be_mount. That's not supposed to be a constant macro. Its just supposed to be that word. > 621-622 Since you are checking to see if a snapshot name exists, it be > clearer > to say something like "0 - success, if snapshot name exists", "1 - Failure, > if > snapshot name doesn't exist" Okay. > 629 zhp_snap is defined, but not used Will remove. Thanks, -ethan > > Sue > >>> Hello again caimaniacs, >>> >>> Thanks for those that provided comments for the early code review. >>> Posted here is the incremental webrev of the changes for code review >>> comments plus other fixes that we've pushed since. >>> >>> http://cr.opensolaris.org/~equach/webrev.SnapUpgrade.2 >>> >>> >>> Additionally as a reference, below is the webrev of the overall snap >>> upgrade gate synced up against the current slim_source base. >>> >>> http://cr.opensolaris.org/~equach/webrev.SnapUpgrade >>> >>> >>> Again comments welcomed, particularly from those who provided the >>> initial comments on the early review. >>> >>> >>> Thanks, >>> -Ethan >>> _______________________________________________ >>> caiman-discuss mailing list >>> caiman-discuss at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >
