On Wed, Jul 19, 2023 at 9:01 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > 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?
There are other places in the code which do something similar by using statically allocated buffers like static char xya[SIZE]. We could do that here. The caller may decide whether to pstrdup this buffer further or just use it one time e.g. as an elog or printf argument. As I said before, we should not even print message type in the error context because it's unknown. Repeating that twice is useless. That will need some changes to apply_error_callback() though. But I am fine with "???" as well. -- Best Wishes, Ashutosh Bapat
