Everything

php

Home » Project » Newsgroups » php.internals » About bug #26753 and zend_register_list_destructors() »

About bug #26753 and zend_register_list_destructors()

php.internals

Jani Taskinen

22 years ago

This is either bug in zend_fetch_list_dtor_id() as said in the
bug report or misuse of [zend_]register_list_destructors().

So which one is it? Is zend_register_list_destructors() deprecated in
favor of zend_register_list_destructors_ex() or what??

FYI: register_list_destructors() is only used by ext/mssql in HEAD.
It's also used by ext/crack in PHP_4_3. But not anywhere else.

What's the difference between these two functions anyway??
(some comments in the sources would be so nice sometimes :)

--Jani

Andi Gutmans

22 years ago

It seems to me that the difference is in the dtor() callback's parameters.
I'm not sure why type_name is not passed to
zend_register_list_destructors(). It probably did not exist when it was
originally written.
I guess we can either fix the comparison, change
zend_register_list_destructors() to accept type_name, or nuke
zend_register_list_destructors() completely and move to _ex().
Probably the last is the best option if we only have to fix two extensions.

Andi

At 11:18 PM 2/15/2004 +0200, Jani Taskinen wrote:

Derick Rethans

22 years ago

On Mon, 16 Feb 2004, Andi Gutmans wrote:

> It seems to me that the difference is in the dtor() callback's parameters.
> I'm not sure why type_name is not passed to
> zend_register_list_destructors(). It probably did not exist when it was
> originally written.
> I guess we can either fix the comparison, change
> zend_register_list_destructors() to accept type_name, or nuke
> zend_register_list_destructors() completely and move to _ex().
> Probably the last is the best option if we only have to fix two extensions.

Third party extensions might use it too, you can't just remove API
functions at will :)

Derick

Andi Gutmans

22 years ago

At 09:25 AM 2/16/2004 +0100, Derick Rethans wrote:

>On Mon, 16 Feb 2004, Andi Gutmans wrote:
>
> > It seems to me that the difference is in the dtor() callback's parameters.
> > I'm not sure why type_name is not passed to
> > zend_register_list_destructors(). It probably did not exist when it was
> > originally written.
> > I guess we can either fix the comparison, change
> > zend_register_list_destructors() to accept type_name, or nuke
> > zend_register_list_destructors() completely and move to _ex().
> > Probably the last is the best option if we only have to fix two extensions.
>
>Third party extensions might use it too, you can't just remove API
>functions at will :)

Uhm, sure you can in a major version especially if it's barely used.

Andi

Jani Taskinen

22 years ago

On Mon, 16 Feb 2004, Andi Gutmans wrote:

>It seems to me that the difference is in the dtor() callback's parameters.
>I'm not sure why type_name is not passed to
>zend_register_list_destructors(). It probably did not exist when it was
>originally written.

Zeev might know? :)

>I guess we can either fix the comparison, change
>zend_register_list_destructors() to accept type_name, or nuke
>zend_register_list_destructors() completely and move to _ex().
>Probably the last is the best option if we only have to fix two extensions.

Wouldn't it be best to fix the comparison in PHP 4 and 5 and
nuke the function in PHP 5? This would let possible 3rd party extensions
use the old macro, but not cause any crashes with the zend_fetch_list_dtor_id()
calls..

And change the extensions that use the register_list_destructors() macro
to use the _ex() function.

--Jani

Andi Gutmans

22 years ago

At 06:03 PM 2/16/2004 +0200, Jani Taskinen wrote:

>On Mon, 16 Feb 2004, Andi Gutmans wrote:
>
> >It seems to me that the difference is in the dtor() callback's parameters.
> >I'm not sure why type_name is not passed to
> >zend_register_list_destructors(). It probably did not exist when it was
> >originally written.
>
>     Zeev might know? :)
>
> >I guess we can either fix the comparison, change
> >zend_register_list_destructors() to accept type_name, or nuke
> >zend_register_list_destructors() completely and move to _ex().
> >Probably the last is the best option if we only have to fix two extensions.
>
>     Wouldn't it be best to fix the comparison in PHP 4 and 5 and
>     nuke the function in PHP 5? This would let possible 3rd party extensions
>     use the old macro, but not cause any crashes with the
> zend_fetch_list_dtor_id()
>     calls..

I don't understand. Why fix it in PHP 5 if you're suggesting to nuke the
function? :)

>
>     And change the extensions that use the register_list_destructors() macro
>     to use the _ex() function.

That can't be done. The callback prototypes are different.

Andi

Jani Taskinen

22 years ago

On Tue, 17 Feb 2004, Andi Gutmans wrote:

>At 06:03 PM 2/16/2004 +0200, Jani Taskinen wrote:
>>On Mon, 16 Feb 2004, Andi Gutmans wrote:
>>
>> >It seems to me that the difference is in the dtor() callback's parameters.
>> >I'm not sure why type_name is not passed to
>> >zend_register_list_destructors(). It probably did not exist when it was
>> >originally written.
>>
>>     Zeev might know? :)
>>
>> >I guess we can either fix the comparison, change
>> >zend_register_list_destructors() to accept type_name, or nuke
>> >zend_register_list_destructors() completely and move to _ex().
>> >Probably the last is the best option if we only have to fix two extensions.
>>
>>     Wouldn't it be best to fix the comparison in PHP 4 and 5 and
>>     nuke the function in PHP 5? This would let possible 3rd party extensions
>>     use the old macro, but not cause any crashes with the
>> zend_fetch_list_dtor_id()
>>     calls..
>
>I don't understand. Why fix it in PHP 5 if you're suggesting to nuke the
>function? :)

I was thinking backwards. Nevermind. So nuke in PHP5, fix in PHP4.

>>     And change the extensions that use the register_list_destructors() macro
>>     to use the _ex() function.
>
>That can't be done. The callback prototypes are different.

Either I'm not getting it or you misunderstood. I meant that the
one extension still in HEAD that uses zend_register_list_destructors()
would be fixed to use the zend_register_list_destructors_ex() function.
(kinda have to, if the former is nuked? :)

--Jani

Andi Gutmans

22 years ago

At 11:36 AM 2/17/2004 +0200, Jani Taskinen wrote:

> >
> >I don't understand. Why fix it in PHP 5 if you're suggesting to nuke the
> >function? :)
>
>     I was thinking backwards. Nevermind. So nuke in PHP5, fix in PHP4.

Yeah I agree. We should commit the fix in PHP 4 and nuke in PHP 5.

> >>     And change the extensions that use the register_list_destructors()
> macro
> >>     to use the _ex() function.
> >
> >That can't be done. The callback prototypes are different.
>
>     Either I'm not getting it or you misunderstood. I meant that the
>     one extension still in HEAD that uses zend_register_list_destructors()
>     would be fixed to use the zend_register_list_destructors_ex() function.
>     (kinda have to, if the former is nuked? :)

That's fine. I thought you meant to make register_list_destructors() call
register_list_destructors_ex().

Andi

Jani Taskinen

22 years ago

On Tue, 17 Feb 2004, Andi Gutmans wrote:

>At 11:36 AM 2/17/2004 +0200, Jani Taskinen wrote:
>
>> >
>> >I don't understand. Why fix it in PHP 5 if you're suggesting to nuke the
>> >function? :)
>>
>>     I was thinking backwards. Nevermind. So nuke in PHP5, fix in PHP4.
>
>Yeah I agree. We should commit the fix in PHP 4 and nuke in PHP 5.
>
>
>> >>     And change the extensions that use the register_list_destructors()
>> macro
>> >>     to use the _ex() function.
>> >
>> >That can't be done. The callback prototypes are different.
>>
>>     Either I'm not getting it or you misunderstood. I meant that the
>>     one extension still in HEAD that uses zend_register_list_destructors()
>>     would be fixed to use the zend_register_list_destructors_ex() function.
>>     (kinda have to, if the former is nuked? :)
>
>That's fine. I thought you meant to make register_list_destructors() call
>register_list_destructors_ex().

It seems that nuking is out of question and not very good idea after all. ;)
Using zend_register_list_destructors() (or the macro, register_list_destructors),
might be valid in some cases and the real bug is only in the zend_fetch_list_dtor_id()
function which really should ignore the 'unnamed resources'.

See ext/soap for more..

--Jani

Andi Gutmans

22 years ago

At 12:01 AM 2/18/2004 +0200, Jani Taskinen wrote:

> >That's fine. I thought you meant to make register_list_destructors() call
> >register_list_destructors_ex().
>
>     It seems that nuking is out of question and not very good idea after
> all. ;)
>     Using zend_register_list_destructors() (or the macro,
> register_list_destructors),
>     might be valid in some cases and the real bug is only in the
> zend_fetch_list_dtor_id()
>     function which really should ignore the 'unnamed resources'.
>
>     See ext/soap for more..

I don't quite understand. Why can't the SOAP module give type_names for
these resources? e.g. "sdl", "url", "service".
What am I missing?

Andi

Jani Taskinen

22 years ago

On Wed, 18 Feb 2004, Andi Gutmans wrote:

>At 12:01 AM 2/18/2004 +0200, Jani Taskinen wrote:
>> >That's fine. I thought you meant to make register_list_destructors() call
>> >register_list_destructors_ex().
>>
>>     It seems that nuking is out of question and not very good idea after
>> all. ;)
>>     Using zend_register_list_destructors() (or the macro,
>> register_list_destructors),
>>     might be valid in some cases and the real bug is only in the
>> zend_fetch_list_dtor_id()
>>     function which really should ignore the 'unnamed resources'.
>>
>>     See ext/soap for more..
>
>I don't quite understand. Why can't the SOAP module give type_names for
>these resources? e.g. "sdl", "url", "service".
>What am I missing?

Those resources are not used anywhere outside the extension..
so why should they be named? (AFAICS, they're never returned anywhere
to user)

And using the zend_register_list_destructors() you can avoid
unnecessary casting from zend_rsrc_list_entry to the desired type.
(I'm of course guessing here :)

--Jani

Andi Gutmans

22 years ago

At 08:20 AM 2/18/2004 +0200, Jani Taskinen wrote:

>On Wed, 18 Feb 2004, Andi Gutmans wrote:
>
> >At 12:01 AM 2/18/2004 +0200, Jani Taskinen wrote:
> >> >That's fine. I thought you meant to make register_list_destructors() call
> >> >register_list_destructors_ex().
> >>
> >>     It seems that nuking is out of question and not very good idea after
> >> all. ;)
> >>     Using zend_register_list_destructors() (or the macro,
> >> register_list_destructors),
> >>     might be valid in some cases and the real bug is only in the
> >> zend_fetch_list_dtor_id()
> >>     function which really should ignore the 'unnamed resources'.
> >>
> >>     See ext/soap for more..
> >
> >I don't quite understand. Why can't the SOAP module give type_names for
> >these resources? e.g. "sdl", "url", "service".
> >What am I missing?
>
>     Those resources are not used anywhere outside the extension..
>     so why should they be named? (AFAICS, they're never returned anywhere
>     to user)

That doesn't matter IMO. I think it's OK if we require people to give a
name to their resources. It will also make it easier to debug their extensions.

>
>     And using the zend_register_list_destructors() you can avoid
>     unnecessary casting from zend_rsrc_list_entry to the desired type.
>     (I'm of course guessing here :)

Yeah but you also get TSRMLS_CC :)

Andi

Wez Furlong

22 years ago

I think ext/soap uses those resources as part of the legacy hack
to store data in an object using integer member names.

The real fix is to port it to PHP 5 OO api and make those things
members of a real C structure.

--Wez.

----- Original Message -----
From: "Andi Gutmans" <andi@zend.com>
To: "Jani Taskinen" <sniper@iki.fi>
Cc: <internals@lists.php.net>
Sent: Wednesday, February 18, 2004 6:22 AM
Subject: Re: [PHP-DEV] Re: About bug #26753 and
zend_register_list_destructors()

> At 08:20 AM 2/18/2004 +0200, Jani Taskinen wrote:
> >On Wed, 18 Feb 2004, Andi Gutmans wrote:
> >
> > >At 12:01 AM 2/18/2004 +0200, Jani Taskinen wrote:
> > >> >That's fine. I thought you meant to make

register_list_destructors() call

> > >> >register_list_destructors_ex().
> > >>
> > >>     It seems that nuking is out of question and not very good idea

after

> > >> all. ;)
> > >>     Using zend_register_list_destructors() (or the macro,
> > >> register_list_destructors),
> > >>     might be valid in some cases and the real bug is only in the
> > >> zend_fetch_list_dtor_id()
> > >>     function which really should ignore the 'unnamed resources'.
> > >>
> > >>     See ext/soap for more..
> > >
> > >I don't quite understand. Why can't the SOAP module give type_names

for

> > >these resources? e.g. "sdl", "url", "service".
> > >What am I missing?
> >
> >     Those resources are not used anywhere outside the extension..
> >     so why should they be named? (AFAICS, they're never returned

anywhere

> >     to user)
>
> That doesn't matter IMO. I think it's OK if we require people to give a
> name to their resources. It will also make it easier to debug their

extensions.

Jani Taskinen

22 years ago

On Wed, 18 Feb 2004, Andi Gutmans wrote:

>At 08:20 AM 2/18/2004 +0200, Jani Taskinen wrote:
>>On Wed, 18 Feb 2004, Andi Gutmans wrote:
>>
>> >At 12:01 AM 2/18/2004 +0200, Jani Taskinen wrote:
>> >> >That's fine. I thought you meant to make register_list_destructors() call
>> >> >register_list_destructors_ex().
>> >>
>> >>     It seems that nuking is out of question and not very good idea after
>> >> all. ;)
>> >>     Using zend_register_list_destructors() (or the macro,
>> >> register_list_destructors),
>> >>     might be valid in some cases and the real bug is only in the
>> >> zend_fetch_list_dtor_id()
>> >>     function which really should ignore the 'unnamed resources'.
>> >>
>> >>     See ext/soap for more..
>> >
>> >I don't quite understand. Why can't the SOAP module give type_names for
>> >these resources? e.g. "sdl", "url", "service".
>> >What am I missing?
>>
>>     Those resources are not used anywhere outside the extension..
>>     so why should they be named? (AFAICS, they're never returned anywhere
>>     to user)
>
>That doesn't matter IMO. I think it's OK if we require people to give a
>name to their resources. It will also make it easier to debug their extensions.

Ok.

>>     And using the zend_register_list_destructors() you can avoid
>>     unnecessary casting from zend_rsrc_list_entry to the desired type.
>>     (I'm of course guessing here :)
>
>Yeah but you also get TSRMLS_CC :)

Heh, that's true. :)

Maybe the first step is to just remove the register_list_destructors macro?
That should give a noticeable heads-up-call for the maintainers of ext/mssql
and ext/soap.. :)

(I can patch ext/mssql, just can't test the changes in action :)

--Jani