On Tue, Jul 18, 2023 at 10:27 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Tue, Jul 18, 2023 at 12:15 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Mon, Jul 17, 2023 at 7:54 PM Alvaro Herrera <alvhe...@alvh.no-ip.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.
