Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:23990
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Message-ID: <003f01c68b13$4f9ce680$88051fac@OHRLVN4523SG>
To: "\"Dmitry Stogov\"" <dmitry@zend.com>
Cc: "Ilia Alshanetsky" <ilia@prohost.org>,
	"Andi Gutmans" <andi@zend.com>,
	"Steph Fox" <steph@zend.com>,
	<internals@lists.php.net>
References: <000001c68af5$50876930$6e02a8c0@thinkpad>
Date: Thu, 8 Jun 2006 08:50:13 -0700
MIME-Version: 1.0
Content-Type: text/plain;
	format=flowed;
	charset="iso-8859-1";
	reply-type=original
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH] Automatic module globals management
From: pollita@php.net ("Sara Golemon")

> The attached patch (for PHP_5_2) implements automatic management of module
> globals.
> The problem that module globals must be unregistered before extension
> unloading, because "globls_dtor" function is unloaded together with
> extension and cannot be called.
>
Dmitry-

  I like the concept especially as it makes this part of the lifecycle-loop 
consistent with the RINIT/SHUTDOWN MINIT/SHUTDOWN design....

  Having said that I like it, here's what I'd change:

*) Nomenclature:  I'd like to see MGCTOR/MGDTOR (and their proto wrappers) 
named more consistently with [RM]INIT/SHUTDOWN. e.g.   GINIT/GSHUTDOWN  or 
TINIT/TSHUTDOWN

*) There are some WS problems in your patch to posix.c

> I really don't like this going into 5_2 branch, and
> the more I think about it the less sure I am about
> it going into HEAD either. It would mean renaming
> all the ctor/dtor functions to the new globals, which
> is going to take an awful lot of #ifdef'ing in PECL
> extensions if they're going to retain compatibility
> across all the current versions of PHP.
>
Steph-

  I think you're missing the fact that Dmitry's patch only *enables* an 
extension to register CTOR/DTORs in the module entry struct.  Existing 
modules can continue to use the manual calls in MINIT/MSHUTDOWN without 
breaking.

-Sara