Skip site navigation (1) Skip section navigation (2)

Re: [COMMITTERS] pgsql: Fix breakage from earlier plperl fix.

From: Alex Hunsaker <badalex(at)gmail(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: [COMMITTERS] pgsql: Fix breakage from earlier plperl fix.
Date: 2012-01-13 02:28:22
Message-ID: CAFaPBrTKKxfA2Vyu3-=GVv-sUn2dph+sdBAGCQEkhTRurvN4MA@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-committerspgsql-hackers
On Fri, Jan 6, 2012 at 14:05, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Alex Hunsaker <badalex(at)gmail(dot)com> writes:
>> Oh my... I dunno exactly what I was smoking last night, but its a good
>> thing I didn't share :-). Uh so my test program was also completely
>> wrong, Ill have to redo it all. I've narrowed it down to:
>>         if ((type == SVt_PVGV || SvREADONLY(sv)))
>>         {
>>             if (type != SVt_PV &&
>>                 type != SVt_NV)
>>             {
>>                 sv = newSVsv(sv);
>>             }
>>        }
>
> Has anyone tried looking at the source code for SvPVutf8 to see exactly
> what cases it fails on?  The fact that there's an explicit croak() call
> makes me think it might not be terribly hard to tell.

Well its easy to find the message, its not so easy to trace it back up
:-). It is perl source code after all. It *looks* like its just:
sv.c:
Perl_sv_pvn_force_flags(SV *sv, STRLEN, I32 flags)
{
 [ Flags is SV_GMAGIC ]
if (SvREADONLY(sv) && !(flags & SV_MUTABLE_RETURN))
   // more or less...
   Perl_croak(aTHX_ "Can't coerce readonly %s to string", ref)

if ((SvTYPE(sv) > SVt_PVLV && SvTYPE(sv) != SVt_PVFM)
            || isGV_with_GP(sv))
            Perl_croak(aTHX_ "Can't coerce %s to string in %s",
sv_reftype(sv,0),
}

Given that I added this hunk:
+
+       if (SvREADONLY(sv) ||
+               isGV_with_GP(sv) ||
+               (SvTYPE(sv) > SVt_PVLV && SvTYPE(sv) != SVt_PVFM))
+               sv = newSVsv(sv);
+       else
+               /* increase the reference count so we cant just
SvREFCNT_dec() it when
+                * we are done */
+               SvREFCNT_inc(sv);

And viola all of these work (both in 5.14 and 5.8.9, although 5.8.9
gives different notices...)

do language plperl $$ elog(NOTICE, *foo); $$;
NOTICE:  *main::foo
CONTEXT:  PL/Perl anonymous code block

do language plperl $$ elog(NOTICE, $^V); $$;
NOTICE:  v5.14.2
CONTEXT:  PL/Perl anonymous code block

do language plperl $$ elog(NOTICE, ${^TAINT}); $$;
NOTICE:  0
CONTEXT:  PL/Perl anonymous code block

So I've done that in the attached patch. ${^TAINT} seemed to be the
only case that gave consistent notices in 5.8.9 and up so I added it
to the regression tests.

Util.c/o not depending on plperl_helpers.h was also throwing me for a
loop so I fixed it and SPI.c...

Thoughts?

Attachment: plperl_ro_v4.patch
Description: text/x-patch (3.6 KB)

In response to

Responses

pgsql-hackers by date

Next:From: Greg SmithDate: 2012-01-13 03:26:12
Subject: Re: checkpoint writeback via sync_file_range
Previous:From: Kevin GrittnerDate: 2012-01-12 23:29:44
Subject: Re: Remembering bug #6123

pgsql-committers by date

Next:From: Fujii MasaoDate: 2012-01-13 04:18:40
Subject: Re: Re: [COMMITTERS] pgsql: Send new protocol keepalive messages to standby servers.
Previous:From: Tom LaneDate: 2012-01-12 21:40:44
Subject: pgsql: Fix CLUSTER/VACUUM FULL for toast values owned byrecently-updat

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group