Re: BUG #13638: Exception texts from plperl has bad encoding - Mailing list pgsql-bugs

From Tom Lane
Subject Re: BUG #13638: Exception texts from plperl has bad encoding
Date
Msg-id 23013.1443457683@sss.pgh.pa.us
Whole thread Raw
In response to Re: BUG #13638: Exception texts from plperl has bad encoding  (Alex Hunsaker <badalex@gmail.com>)
Responses Re: BUG #13638: Exception texts from plperl has bad encoding  (Alex Hunsaker <badalex@gmail.com>)
List pgsql-bugs
Alex Hunsaker <badalex@gmail.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

pgsql-bugs by date:

Previous
From: Tom Lane
Date:
Subject: Re: BUG #13646: Upgrading existing db from 9.2 to 9.4.4 not working using postgresql-setup.
Next
From: digoal@126.com
Date:
Subject: BUG #13645: pg_basebackup backup hash index & unlogged table