Re: [PATCH] Reference leak in `iprin1 ()'

[Neil Jerram]

address@hidden (Ludovic Courtès) writes:

> Hello,
>
> Neil Jerram <address@hidden> writes:
>
>> Nice work, but it looks to me that PUSH_REF sets the value of the
>> (pstate->top)th element _before_ incrementing pstate->top.  So
>> shouldn't your fix do the decrement first and then set the slot to
>> undefined?
>
> Yes, you're perfectly right.

Thanks.  (It's always better to have other people check and agree!)

>> Also, is the () -> (void) change strictly related?
>
> In fact, it's strictly unrelated, but while we're at it (really `(void)'
> was meant here, not `()')...

Why?  Does () have any symptoms that we should be concerned about?

> Below is an updated patch.  I modified `PUSH_REF ()' as well so that it
> does PSTATE->TOP++ _after_ using the `PSTATE_STACK_SET ()' macro: this
> is safer.

Why is it safer?  My inclination is that it's usually safer to change
less code, other things being equal.

>  And, well, I couldn't resist the desire to "beautifully
> backslashify" `ENTER_NESTED_DATA ()' as well.  :-)  I hope this is not
> too much pollution for you.

No, I'm happy to go along with that one!

Regards,
        Neil