Re: BUG #13638: Exception texts from plperl has bad encoding

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alex Hunsaker <badalex(at)gmail(dot)com>
Cc: Tim Bunce <Tim(dot)Bunce(at)pobox(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, lei(at)aswsyst(dot)cz, pgsql-bugs <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: BUG #13638: Exception texts from plperl has bad encoding
Date: 2015-09-28 16:28:03
Message-ID: 23013.1443457683@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Alex Hunsaker <badalex(at)gmail(dot)com> writes:
> Yeah, I was just looking at that. I couldn't find a good way to support
> 5.8. Doing: ...
> Works, but screws up the error line numbers.

I looked into this and determined that the problem is that the location
info doesn't get appended to ERRSV until after Perl has popped the stack.

A possibly-entirely-wrong hack that seems to fix that is to use mess()
to construct the SV, *and* append the location info, before we croak.
I tried this implementation of croak_cstr:

char *utf8_str = utf_e2u(str);
SV *ssv = mess("%s", utf8_str);
SV *errsv = get_sv("@", GV_ADD);

SvUTF8_on(ssv);
pfree(utf8_str);

sv_setsv(errsv, ssv);
croak(NULL);

with Perl 5.8.9 and it passed your proposed regression test. Obvious
questions include:

* I don't see mess() in the perlapi man page, so is it okay to use?

* Is there any leakage involved in this? (I don't know what prompted
you to add sv_2mortal() to the other implementation, but maybe we need
it here too?)

* Is there some other reason why this is wrong or a bad idea? Since we'd
only use this with versions lacking croak_sv(), future-proofing doesn't
seem like a good argument against it, but maybe there is another one.

BTW, I think we can't actually use this regression test case, because it
won't work as-is in non-UTF8 encodings. But the use of UTF8 characters in
the string doesn't seem like an essential property of the test. However,
other test cases may already provide sufficient coverage if we're not
going to test encoding conversion.

Also, I'm inclined to leave do_util_elog() alone other than replacing
croak("%s", edata->message) with croak_cstr(edata->message). Since
we have to have croak_cstr() anyway, there seems little value in taking
any risk that we've missed some reason why the existing coding there
is important.

regards, tom lane

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Jeremy Whiting 2015-09-28 18:51:56 Re: BUG #13646: Upgrading existing db from 9.2 to 9.4.4 not working using postgresql-setup.
Previous Message Tom Lane 2015-09-28 13:38:37 Re: BUG #13646: Upgrading existing db from 9.2 to 9.4.4 not working using postgresql-setup.