Everything

php

fix for bug #26696

php.internals

Sterling Hughes

22 years ago

Hey,

I've attached a fix for an engine level crash in switch() when it is
used with string offsets.  This eliminates a double-free and valgrind
reports that no new leaks have been introduced.

-Sterling

Andi Gutmans

22 years ago

Are you sure this is OK? It seems strange that string offsets don't need
any unlocking. It might mean we have a problem someplace else and that
you're fixing the cause and not the source of the problem (although I
haven't had time to dig deep).
In general, consider the following code:
$a = "Foo";

switch ($a[0]) {
case 'b':
break;
case 'c':
break;
case 'F':
break;
}

Are you sure this doesn't leak now? I don't have time to look at it now,
but if you are absolutely convinced the patch makes sense I'll try and step
through it with a debugger as soon as I have some free time.

BTW, thanks for taking a look at it.
Andi

At 03:03 PM 1/13/2004 -0500, Sterling Hughes wrote:

Sterling Hughes

22 years ago

> Are you sure this is OK? It seems strange that string offsets don't need
> any unlocking. It might mean we have a problem someplace else and that
> you're fixing the cause and not the source of the problem (although I
> haven't had time to dig deep).
> In general, consider the following code:
> $a = "Foo";
>
> switch ($a[0]) {
>         case 'b':
>                 break;
>         case 'c':
>                 break;
>         case 'F':
>                 break;
> }
>
> Are you sure this doesn't leak now? I don't have time to look at it now,
> but if you are absolutely convinced the patch makes sense I'll try and step
> through it with a debugger as soon as I have some free time.
>

No, I was wrong.  I hate the memory manager, it cleans up my leaks for
me - makes it impossible to rely on valgrind.  ;) Anyway to turn that off?

I'm playing around some more now.

-Sterling

Andi Gutmans

22 years ago

At 04:06 PM 1/13/2004 -0500, Sterling Hughes wrote:

> > Are you sure this is OK? It seems strange that string offsets don't need
> > any unlocking. It might mean we have a problem someplace else and that
> > you're fixing the cause and not the source of the problem (although I
> > haven't had time to dig deep).
> > In general, consider the following code:
> > $a = "Foo";
> >
> > switch ($a[0]) {
> >         case 'b':
> >                 break;
> >         case 'c':
> >                 break;
> >         case 'F':
> >                 break;
> > }
> >
> > Are you sure this doesn't leak now? I don't have time to look at it now,
> > but if you are absolutely convinced the patch makes sense I'll try and
> step
> > through it with a debugger as soon as I have some free time.
> >
>
>No, I was wrong.  I hate the memory manager, it cleans up my leaks for
>me - makes it impossible to rely on valgrind.  ;) Anyway to turn that off?

If you're running in --enable-debug it should be telling you about the leaks.

>I'm playing around some more now.

OK thanks.

Andi

Sterling Hughes

22 years ago

Ok, attached is a diff that properly fixes the problem.

-Sterling

Jon Parise

22 years ago

On Tue, Jan 13, 2004 at 05:27:05PM -0500, Sterling Hughes wrote:

> Ok, attached is a diff that properly fixes the problem.

[..]

>  				/* perform the equivalent of equivalent of a

While you're in there, could you fix the double occurrence double
occurrence in this comment?

--
Jon Parise (jon@php.net) :: The PHP Project (http://www.php.net/)