On Wed, Jun 10, 2009 at 2:18 PM, Jerry Jelinek <[email protected]> wrote: > > I need another code review for the p2v support for > zones using IPS. This is bug: > > 6793 p2v support for ipkg-branded zones
I think I reviewed this before, but with another look I've found a few items. The only one that is likely to cause problems in a sane environment is the last one. The rest are probably nits for things that would (at worst) be problematic only if files or directories have white space or PATH has buggy utilities ahead of /usr/bin. image_install general: - there seems to be inconsistent use of variable quoting within [[ ]] - variables should always be quoted when put on the command line 26 - 31: should probably move PATH setting ahead of ". <file>" so that commands in the included files (e.g. gettext) don't come from the wrong place. 40, 42, 50, 88, ..: sometimes "error" sometimes "ERROR" 166-193: most variables are unset before getopt, but install_media, preserve_zone, unconfig_zone aren't 233: avoid fork with: if [[ "$install_media" != /* ]] 403-418: I would use "case $filetype in ... esac" instead of "if elif elif ... fi" 498: s/0/$ZONE_SUBPROC_OK/ ? p2v General: - either set PATH or specify complete path to commands (e.g. calls to gettext, lines 218, 629, 632, etc.). Setting PATH and not specifying full paths to commands allows for use of built-ins as more are added to ksh. 533: What about TERM, HUP, etc? Perhaps EXIT would be best... src/pkgdefs/SUNWipkg-brand/prototype 6-7: *.conf files have comments that look like they should be type e not f. -- Mike Gerdts http://mgerdts.blogspot.com/ _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
