On Tue, Feb 8, 2011 at 10:33, Alex Hunsaker <badalex@gmail.com> wrote:
> On Tue, Feb 8, 2011 at 08:18, Alexey Klyukin <alexk@commandprompt.com> wrote:
>>
>> On Feb 6, 2011, at 9:43 AM, Alex Hunsaker wrote:
>
>>> So here is a v6
>>> ....
>>> Comments?
>>
>> Thanks, looks great to me. It passes all the tests on my OS X system. I wonder
>> what's the purpose of the amagic_call in get_perl_array_ref, instead of
>> calling newRV_noinc on the target SV * ?
>
> Err, even simpler would be to just access the 'array' member directly
out of the hash object.
Done as the above, an added bonus is we no longer have to SvREF_dec()
so its even simpler.
>> Also, in array_to_datum (line 1050), if get_perl_array_ref returns NULL for
>> the first element of the corresponding dimension, but non-NULL for the second
>> one - it would use uninitialized dims[cur_depth] value in comparison (which,
>> although, would probably lead to the desired comparison result).
>
> Good catch, will fix. All of those checks should be outside of the while loop.
I clearly needed more caffeine in my system when i wrote that. Partly
due to the shadowed "av" variable. I've fixed it by initiating all of
the dims to 0. I also renamed the shadowed av var to "nav" for nested
av.
> While Im at it Ill also rebase against HEAD (im sure there will be
> some conflicts with that other plperl patch that just went in ;)).
So the merge while not exactly trivial was fairly simple. However it
would be great if you could give it another look over.
Find attached v7 changes include:
- rebased against HEAD
- fix potential use of uninitialized dims[cur_depth]
- took out accidental (broken) hack to try and support composite types
in ::encode_array_literal (added in v4 or something)
- make_array_ref() now uses plperl_hash_from_datum() for composite
types instead of its own hand rolled version
- get_perl_array_ref() now grabs the 'array' directly instead of
through the magic interface for simplicity
- moved added static declarations to the "bottom" instead of being
half on top and half on bottom