Re: logicalrep_message_type throws an error

On Tue, Jul 18, 2023 at 10:27 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Tue, Jul 18, 2023 at 12:15 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Mon, Jul 17, 2023 at 7:54 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> > >
> >
> > 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?
>

In the above size calculation, shouldn't it be 7 + 11 where 7 is for
(3 (???) + 1 for space + 2 for () + 1 for terminating null char) and
11 is for %d? BTW, this avoids dynamic allocation of the err string in
logicalrep_message_type() but we can't return a locally allocated
string, so do you think we should change the prototype of the function
to get this as an argument and then use it both for valid and invalid
cases? I think if there is some simpler way to achieve this then fine,
otherwise, let's return a constant string like "???" from
logicalrep_message_type() for the invalid action.

--
With Regards,
Amit Kapila.