public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Kapila <[email protected]>
To: Ashutosh Bapat <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Euler Taveira <[email protected]>
Cc: [email protected]
Subject: Re: logicalrep_message_type throws an error
Date: Thu, 20 Jul 2023 09:10:45 +0530
Message-ID: <CAA4eK1JQB=musr4+HA_3WvnUYrRsir2zQ2Ks0au+Za3y_kQqMQ@mail.gmail.com> (raw)
In-Reply-To: <CAExHW5uiKkfW8b6uLuAqjsiiGLTeU2ZRpPkPfzhwrpUmHT3SAA@mail.gmail.com>
References: <CAExHW5vvhhtdv-LctC5Wz-yuyh1me3KtvyuOqX48BRQicDGTJg@mail.gmail.com>
<[email protected]>
<CAA4eK1+A3h9rmboKA9UH_Fzbw+zJax1WPxgtme=kfvp-8chG6w@mail.gmail.com>
<CAD21AoC0zZqxHPb44akZg2+SzkUrrG23p01NvK=Binkc67sCeg@mail.gmail.com>
<CAA4eK1KG9C5vkjQSVi-omT1QJsDoZDnBEiY+rVfNQbqKz0UtYw@mail.gmail.com>
<CAExHW5uiKkfW8b6uLuAqjsiiGLTeU2ZRpPkPfzhwrpUmHT3SAA@mail.gmail.com>
On Wed, Jul 19, 2023 at 10:08 AM Ashutosh Bapat
<[email protected]> wrote:
>
> On Wed, Jul 19, 2023 at 9:01 AM Amit Kapila <[email protected]> wrote:
> >
> > On Tue, Jul 18, 2023 at 10:27 AM Masahiko Sawada <[email protected]> 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.
view thread (10+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: logicalrep_message_type throws an error
In-Reply-To: <CAA4eK1JQB=musr4+HA_3WvnUYrRsir2zQ2Ks0au+Za3y_kQqMQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox