|From:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|To:||Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>|
|Cc:||PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>|
|Subject:||Re: Allowing printf("%m") only where it actually works|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
> Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> writes:
>> Here's an experimental way to do that, if you don't mind depending on
>> gory details of libc implementations (ie knowledge of what it expands
>> too). Not sure how to avoid that since it's a macro on all modern
>> systems, and we don't have a way to temporarily redefine a macro. If
>> you enable it for just ereport(), it compiles cleanly after 81256cd
>> (but fails on earlier commits). If you enable it for elog() too then
>> it finds problems with exec.c.
> Hmm ... that's pretty duff code in exec.c, isn't it. Aside from the
> question of errno unsafety, it's using elog where it really ought to be
> using ereport, it's not taking any thought for the reported SQLSTATE,
> etc. I'm hesitant to mess with it mere hours before the beta wrap,
> but we really oughta improve that.
I wrote up a patch that makes src/common/exec.c do error reporting more
like other frontend/backend-common files (attached). Now that I've done
so, though, I'm having second thoughts. The thing that I don't like
about this is that it doubles the number of translatable strings created
by this file. While there's not *that* many of them, translators have to
deal with each one several times because this file is included by several
different frontend programs. So that seems like a rather high price to
pay to deal with what, at present, is a purely hypothetical hazard.
(Basically what this would protect against is elog_start changing errno,
which it doesn't.) Improving the errcode situation is somewhat useful,
but still maybe it's not worth the cost.
Another approach we could consider is keeping exec.c's one-off approach
to error handling and letting it redefine pg_prevent_errno_in_scope() as
empty. But that's ugly.
Or we could make the affected call sites work like this:
int save_errno = errno;
log_error(_("could not identify current directory: %s"),
which on the whole might be the most expedient thing.
regards, tom lane
|Next Message||Tom Lane||2018-05-26 16:54:45||Re: Adding a new table to the system catalog|
|Previous Message||Chapman Flack||2018-05-26 15:44:19||Re: SPI/backend equivalent of extended-query Describe(statement)?|