| From: | Andres Freund <andres(at)anarazel(dot)de> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Odd usage of errmsg_internal in bufmgr.c |
| Date: | 2026-02-12 02:19:32 |
| Message-ID: | aY04HdoG2PaXpOE6@alap3.anarazel.de |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On 2026-02-11 20:34:50 -0500, Tom Lane wrote:
> Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> writes:
> > The relevant code looks like this:
>
> > msg_one = _("invalid page in block %u of relation \"%s\””);
>
> > ereport(elevel,
> > errcode(ERRCODE_DATA_CORRUPTED),
> > errmsg_internal(msg_one, first + first_off, rpath.str) :
>
> I agree that that isn't great code, but what I don't like about it
> is the separation between where the format string is defined and
> where it is used. It'd be very easy for the %-escapes to get out of
> sync with the types of the actual parameters, and if they did, the
> compiler would not warn you. I think we ought to try to recast this
> into the normal usage pattern where the format is literal within the
> errmsg call. I see the comment about avoiding code duplication, but
> to my mind this is a terrible solution.
The amount of code duplication it avoids is rather substantial. Yes, it's not
great to loose the compiler checking for format codes, but each of the
branches is actually, so the likelihood of that not being noticed doesn't seem
significant.
Greetings,
Andres Freund
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Zhang Mingli | 2026-02-12 02:19:42 | Re: Regression failures after changing PostgreSQL blocksize |
| Previous Message | Shinya Kato | 2026-02-12 02:05:42 | Re: remove unnecessary include in src/backend/commands/policy.c |