Hi Keith, Looks ok.
Sue 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
