On Tue, Jul 18, 2023 at 12:15 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Jul 17, 2023 at 7:54 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> >
> > On 2023-Jul-17, Ashutosh Bapat wrote:
> >
> > > Prologue of psprintf() says
> > >
> > > * Errors are not returned to the caller, but are reported via elog(ERROR)
> > > * in the backend, or printf-to-stderr-and-exit() in frontend builds.
> > > * One should therefore think twice about using this in libpq.
> > >
> > > If an error occurs in psprintf(), it will throw an error which will
> > > override the original error. I think we should avoid any stuff that
> > > throws further errors.
> >
> > Ooh, yeah, this is an excellent point. I agree it would be better to
> > avoid psprintf() here and anything that adds more failure modes.
> >
>
> I have tried to check whether we have such usage in any other error
> callbacks. Though I haven't scrutinized each and every error callback,
> I found a few of them where an error can be raised. For example,
>
> rm_redo_error_callback()->initStringInfo()
> CopyFromErrorCallback()->limit_printout_length()
> shared_buffer_write_error_callback()->relpathperm()->relpathbackend()->GetRelationPath()->psprintf()
>
> > Let's
> > just do the thing in the original patch you submitted, to ensure all
> > these strings are compile-time constants; that's likely the most robust.
> >
Or can we use snprintf() writing "??? (%d)" to a fixed length char[8 +
11] allocated on the stack instead?
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com