On Wed, Jul 19, 2023 at 10:08 AM Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > > 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: > > > > > > 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. >
Okay, changed it accordingly. Currently, we call it only from errcontext, so it looks reasonable to me to use static here. > 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. > I think in the end it won't make a big difference. So, I would like to go with Sawada-San's suggestion to keep the message type consistent in actual error and error context unless that requires bigger changes. -- With Regards, Amit Kapila.