On Tue, Mar 24, 2015 at 1:08 PM, Dmitry Stogov <dmi...@zend.com> wrote:
>
>
> On Tue, Mar 24, 2015 at 2:51 PM, Nikita Popov <nikita....@gmail.com>
> wrote:
>
>> On Tue, Mar 24, 2015 at 10:28 AM, Dmitry Stogov <dmi...@zend.com> wrote:
>>
>>> I thought about something like this :)
>>> In my opinion UString is really not a proper way to implement Unicoide,
>>> but I agree not break anything in current stage.
>>> Anyway, please review the first PR (in my opinion it is safe to commit),
>>> but you may find some other issues.
>>>
>>> Thanks. Dmitry.
>>>
>>
>> First PR looks okay to me. One question: Why the separate INIT and ADD
>> opcodes? They seem pretty much the same, just one using a hardcoded 0
>> instead of ex_val.
>>
>
> Oh. It's historical, at first we allocated rope on heap. I'll check if
> INIT_ROPE is still necessary.
> Most probably we still need it, to know the size of rope vector.
>
>
>> Regarding exception-safety - is the problem that doing an EG(exception)
>> check and releasing the rope is too expensive?
>>
>
> No. It's a common problem. Because exception may be thrown in some other
> opcode and we don't clean IS_VAR/IS_TMP_VAR zvals.
> For example the following script leaks independently from the patch
>
> <?php
> function foo() {
> throw new Exception();
> }
> try {
> $a = "a";
> $a = $a . $a . $a . foo() . $a . "\n";
> } catch (Exception $e) {
> }
> ?>
>
Ah yes, I forgot about this issue. However, shouldn't we at least properly
release memory if the exception occurs in the opcode?
Nikita
