webrev has been updated.
ed
On Mon, Jun 27, 2011 at 10:13:31AM -0500, Mike Gerdts wrote:
> I've only looked at the ksh stuff.
>
> src/brand/attach
>
> 305, 317, possibly others: Please use tabs instead of spaces.
>
um. i did use tabs.
> Rather than:
>
> pkg_args=""
> ...
> pkg_args="$pkg_args --foo $mumble"
> pkg_args="$pkg_args $something"
> $PKG $pkg_args
>
> How about:
>
> typeset -a pkg_args
> ...
> a_push pkg_args --foo "$mumble"
> a_push pkg_args "$something"
> $PKG "${pkg_args[@]}"
>
> Because each argument to $PKG is now quoted, we avoid:
>
> - Problems with white space in args. (There's an open CR about
> dealing with spaces in paths and/or dataset names. Not
> necessarily relevent here, but this is the type of thing that
> would need to be fixed for that.)
> - In the original, if $mumble is null, $something becomes the
> argument to --foo. With the fix, a null string becomes the
> argument to --foo.
>
grumble, grumble. i really dislike the advanced shell syntax when it's
not necessary since it just makes things harder to understand for mere
mortals. in this case i don't think it's really justified because the
optional parameters that i add are string literals (not expansions of
some other variables) that don't have any spaces in them. so for
readability sake i'd prefer to leave this as is. when someone adds a
third parameter that might need spaces they can switch to some other
syntax.
>
> p2v
>
> 299: If /etc/zones/index is a dangling symlink, do we want a error
> or to return 0? Why return 0 rather than creating an empty file?
>
that file should really never be a symlink. if the user is manually
replacing random files with symlinks i think it's ok to generate an
error here. just don't do that.
> Perhaps we should be handling zones that are "incomplete" as well as
> installed. The best way to deal with this would probably be to
> change the check at 311 & 324 to:
>
> if ($2 != "configured")
>
done.
> On Tue 21 Jun 2011 at 07:06PM, Edward Pilatowicz wrote:
> > hey all,
> >
> > i've got another wad of misc zones/li bugfixes:
> >
> > http://mcescher.us.oracle.com/export/ws/pkg.bugfix2/webrev
> >
> > 18349 linked image code needs to be more resilient in the face of zoneadm
> > failure
> > 18550 zone attach should reject entire if necessary
> > 18578 p2v - cannot install s11 images with embedded zones
> >
> > thanks
> > ed
>
> --
> Mike Gerdts
> Solaris Core OS / Zones
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
