Thanks Karen
On 03/ 2/11 11:04 AM, Karen Tung wrote:
Looks good to me Keith.
--Karen
On 03/ 2/11 10:19 AM, Keith Mitchell wrote:
Hi all,
Can I get a code review of my changes for the following bugs, please?
7023850 Fix for 7020660 needed to updated enable_install_service
in installadm
7023644 solaris_install.Popen.__del__() sometimes has problems
Webrev:
http://cr.opensolaris.org/~kemitche/webrev.7023850
Details:
7023850 - When I fixed and pushed 7020660, I made an incorrect
assumption that there were no consumers of that code yet, so the
change wouldn't break anything. There was one affected consumer, and
the one line change in ai_smf_service fixes that consumer. I've also
verified that there are no other consumers of solaris_install.Popen
that were affected.
7023644 - The __del__ code was actually somewhat broken and
unnecessary. First, in some cases, the destructor would be called in
such a way that the super() call would fail - this never caused an
actual problem, but it generated confusing noise to stderr:
Exception TypeError: 'super() argument 1 must be type, not None' in
<bound method Popen.__del__ of <solaris_install.Popen object at
0x848560c>> ignored
The __del__ code is actually unnecessary. It attempted to close the
filehandles to /dev/null when the Popen object lost scope, but (1)
the parent Popen class does this in most cases, and (2) any cases
where it doesn't, the __del__ methods of the open filehandles get
called anyway, so they close themselves without my "help".
- Keith
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
