[PATCH] =& doesn't remove "old" references like unset()

php.internals

Matt W

19 years ago
Hi (Dmitry?), See Bug #33282 -- I saw it in the Bug Summary; don't know if there are other related ones... Same applies to foreach ($arr ... &$v) which is where I noticed it last week with var_dump($arr). All elements that WERE referenced but aren't anymore still have is_ref=1 (when refcount=1). Only the last referenced element should be a reference (unless the others were to begin with). unset()'ing the reference variable removes the reference from the last element. Example: $a = array(1, 2, 3); $r = &$a[0]; $r = &$a[1]; $r = &$a[2]; var_dump($a); array(3) { [0]=> &int(1) [1]=> &int(2) [2]=> &int(3) // unset($r) will take care of this one } The reference (&) should no longer be on the first 2 elements, right? Setting is_ref=0 when refcount=1 in zend_assign_to_variable_reference() fixes it. I assume it won't cause other problems since the same thing is done in zval_ptr_dtor(). :-) Thanks, Matt

Michael Wallner

19 years ago
Matt Wilmas wrote:
> Hi (Dmitry?), > > See Bug #33282 -- I saw it in the Bug Summary; don't know if there are other > related ones... Same applies to foreach ($arr ... &$v) which is where I > noticed it last week with var_dump($arr). All elements that WERE referenced > but aren't anymore still have is_ref=1 (when refcount=1). Only the last > referenced element should be a reference (unless the others were to begin > with). unset()'ing the reference variable removes the reference from the > last element. Example: > > $a = array(1, 2, 3); > $r = &$a[0]; > $r = &$a[1]; > $r = &$a[2]; > var_dump($a); > > array(3) { > [0]=> > &int(1) > [1]=> > &int(2) > [2]=> > &int(3) // unset($r) will take care of this one > } > > The reference (&) should no longer be on the first 2 elements, right? > Setting is_ref=0 when refcount=1 in zend_assign_to_variable_reference() > fixes it. I assume it won't cause other problems since the same thing is > done in zval_ptr_dtor(). :-) >
This looks like it may actually fix a lot of previously bogus'ed bug reports?
> > ------------------------------------------------------------------------ > > Index: zend_execute.c > =================================================================== > RCS file: /repository/ZendEngine2/zend_execute.c,v > retrieving revision 1.752 > diff -u -r1.752 zend_execute.c > --- zend_execute.c 2 Oct 2006 11:05:02 -0000 1.752 > +++ zend_execute.c 7 Nov 2006 05:24:38 -0000 > @@ -438,6 +438,8 @@ > if (variable_ptr->refcount==0) { > zendi_zval_dtor(*variable_ptr); > FREE_ZVAL(variable_ptr); > + } else if (variable_ptr->refcount == 1) { > + variable_ptr->is_ref = 0; > } > } else if (!variable_ptr->is_ref) { > if (variable_ptr_ptr == value_ptr_ptr) { > > > ------------------------------------------------------------------------ > > Index: zend_execute.c > =================================================================== > RCS file: /repository/ZendEngine2/zend_execute.c,v > retrieving revision 1.716.2.12.2.12 > diff -u -r1.716.2.12.2.12 zend_execute.c > --- zend_execute.c 2 Oct 2006 11:09:52 -0000 1.716.2.12.2.12 > +++ zend_execute.c 7 Nov 2006 05:24:40 -0000 > @@ -415,6 +415,8 @@ > if (variable_ptr->refcount==0) { > zendi_zval_dtor(*variable_ptr); > FREE_ZVAL(variable_ptr); > + } else if (variable_ptr->refcount == 1) { > + variable_ptr->is_ref = 0; > } > } else if (!variable_ptr->is_ref) { > if (variable_ptr_ptr == value_ptr_ptr) {
-- Michael

Matt W

19 years ago
----- Original Message ----- From: "Michael Wallner" Sent: Tuesday, November 07, 2006
> Matt Wilmas wrote: > > Hi (Dmitry?), > > > > See Bug #33282 -- I saw it in the Bug Summary; don't know if there are
other
> > related ones... Same applies to foreach ($arr ... &$v) which is where I > > noticed it last week with var_dump($arr). All elements that WERE
referenced
> > but aren't anymore still have is_ref=1 (when refcount=1). Only the last > > referenced element should be a reference (unless the others were to
begin
> > with). unset()'ing the reference variable removes the reference from
the
> > last element. Example: > > > > $a = array(1, 2, 3); > > $r = &$a[0]; > > $r = &$a[1]; > > $r = &$a[2]; > > var_dump($a); > > > > array(3) { > > [0]=> > > &int(1) > > [1]=> > > &int(2) > > [2]=> > > &int(3) // unset($r) will take care of this one > > } > > > > The reference (&) should no longer be on the first 2 elements, right? > > Setting is_ref=0 when refcount=1 in zend_assign_to_variable_reference() > > fixes it. I assume it won't cause other problems since the same thing
is
> > done in zval_ptr_dtor(). :-) > > > > This looks like it may actually fix a lot of previously bogus'ed bug
reports? Not sure which ones you mean (searching for "reference" returns many bugs, and I didn't look very thoroughly), but so everyone knows since I mentioned foreach(), this doesn't change the behavior reported in Bugs #36240, #37410, #39307, etc...
> -- > Michael
Matt

Dmitry Stogov

19 years ago
Hi Michael, I think the patch is correct. Really you can just call zval_ptr_dtor() to make the magic in one place. Thanks. Dmitry.

Michael Wallner

19 years ago
Dmitry Stogov wrote:
> Hi Michael, > > I think the patch is correct. > Really you can just call zval_ptr_dtor() to make the magic in one place.
Someone with Zend/ karma needs to take care of it then. Regards,
-- Michael

Dmitry Stogov

19 years ago
OK. I'll care about it. Not sure if I'll have time tomorrow, but I'll try not to delay it a lot. Thank you. Dmitry.

Ilia A.

19 years ago
Dmitry, I've applied the adjusted patch. On 7-Nov-06, at 2:08 PM, Dmitry Stogov wrote:
> OK. I'll care about it. > Not sure if I'll have time tomorrow, but I'll try not to delay it a > lot. > Thank you. > > Dmitry. > >> -----Original Message----- >> From: Michael Wallner [mailto:mike@php.net] >> Sent: Tuesday, November 07, 2006 6:54 PM >> To: internals@lists.php.net; Dmitry Stogov >> Subject: Re: [PHP-DEV] Re: [PATCH] =& doesn't remove "old" >> references like unset() >> >> >> Dmitry Stogov wrote: >>> Hi Michael, >>> >>> I think the patch is correct. >>> Really you can just call zval_ptr_dtor() to make the magic in one >>> place. >> >> Someone with Zend/ karma needs to take care of it then. >> >> Regards, >> -- >> Michael >> >> -- >> PHP Internals - PHP Runtime Development Mailing List >> To unsubscribe, visit: http://www.php.net/unsub.php >> > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >
Ilia Alshanetsky