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
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-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 Content-Type Size
plperl_ro_v4.patch text/x-patch 3.6 KB

In response to

Responses

Browse pgsql-committers by date

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

Browse pgsql-hackers by date

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