Hi,

On 2026-02-11 20:34:50 -0500, Tom Lane wrote:
> Chao Li <[email protected]> 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


Reply via email to