Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:18896
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Date: Wed, 14 Sep 2005 16:02:08 +0200
Reply-To: Marcus Boerger <helly@php.net>
Message-ID: <1076283310.20050914160208@marcus-boerger.de>
To: Rasmus Lerdorf <rasmus@lerdorf.com>
Cc: internals <internals@lists.php.net>
In-Reply-To: <432534E4.9020602@lerdorf.com>
References: <432534E4.9020602@lerdorf.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] ref fix revisited
From: helly@php.net (Marcus Boerger)

Hello Rasmus,

  my solution was (and i proposed a patch for it here) that we have a way
to allow function signatures that pass variables as const just like c++
allows. This i did because first it is faster and second it applies to
most pass by ref signatures. Or in other words we could get rid of most
pass by ref sigs. And just to note, it was declined because it would
increase complexity a tight bit.

best regards
marcus

Monday, September 12, 2005, 9:57:24 AM, you wrote:

> Guys, could we take a look at making the ref to temp var fix a bit
> narrower?  Currently we try to catch it at call-time.  This means that
> something like:

>   current(explode(' ','a b'))

> as per bug #34468 doesn't work.  Now, I think there is a secondary bug
> here.  I see no reason for current() to take a by-ref, so this
> particular one could be easily fixed.  But there are many other cases
> where a function legitimately takes a by-ref and doesn't necessarily
> write to it or the write is a secondary action not required for the code
> to work.  Could we not catch this on the write instead of on the call?
> The memory problem happens on the write.  Or perhaps better, an E_NOTICE
> or E_STRICT on the call and an E_FATAL on the write.  The current
> E_FATAL on the call seems out of whack.

> Gallery, for example, broke in a rather subtle way in their
> gallery_remote2.php script which meant the various client-side tools
> like iphototogallery and others got a cryptic "no album at URL" error
> message.  I had to break out ethereal to track it down to a couple of
> functions where read-only function args were marked as by-ref.  So they
> didn't actually have a memory corruption problem yet the E_FATAL killed
> them.

> SquirrelMail has code like this all over the place:

>    $value = strtolower(array_shift(split('/\w/',trim($value))));

> Here array_shift() does of course change the arg, so that is a potential
> problem.  And yes, that's a dumb way to do this, but people write code
> like this.  In some of these array manipulation calls, which seems to
> account for a number of the BC problems we are having, we could check
> for a non-ref and behave slightly differently.  In the case of
> array_shift() we could return the first arg and throw a notice.  Same
> would go for reset(), end(), next(), prev() and probably a few others.

> -Rasmus




Best regards,
 Marcus