MIME-Version: 1.0
References: <CAExHW5vvhhtdv-LctC5Wz-yuyh1me3KtvyuOqX48BRQicDGTJg@mail.gmail.com>
 <20230717142452.74yml7mfqkypfixp@alvherre.pgsql> <CAA4eK1+A3h9rmboKA9UH_Fzbw+zJax1WPxgtme=kfvp-8chG6w@mail.gmail.com>
 <CAD21AoC0zZqxHPb44akZg2+SzkUrrG23p01NvK=Binkc67sCeg@mail.gmail.com>
In-Reply-To: <CAD21AoC0zZqxHPb44akZg2+SzkUrrG23p01NvK=Binkc67sCeg@mail.gmail.com>
From: Amit Kapila <amit.kapila16@gmail.com>
Date: Wed, 19 Jul 2023 09:01:34 +0530
Message-ID: <CAA4eK1KG9C5vkjQSVi-omT1QJsDoZDnBEiY+rVfNQbqKz0UtYw@mail.gmail.com>
Subject: Re: logicalrep_message_type throws an error
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, 
	Euler Taveira <euler@eulerto.com>, pgsql-hackers@lists.postgresql.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAA4eK1KG9C5vkjQSVi-omT1QJsDoZDnBEiY%2BrVfNQbqKz0UtYw%40mail.gmail.com>
Precedence: bulk

On Tue, Jul 18, 2023 at 10:27=E2=80=AFAM Masahiko Sawada <sawada.mshk@gmail=
.com> wrote:
>
> On Tue, Jul 18, 2023 at 12:15=E2=80=AFPM Amit Kapila <amit.kapila16@gmail=
.com> wrote:
> >
> > On Mon, Jul 17, 2023 at 7:54=E2=80=AFPM Alvaro Herrera <alvherre@alvh.n=
o-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 robu=
st.
> > >
>
> 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.

--=20
With Regards,
Amit Kapila.