Date: Thu, 09 Jul 2026 15:15:40 +0900 (JST)
Message-Id: <20260709.151540.712205027352253675.horikyota.ntt@gmail.com>
To: gonzalemario@gmail.com
Cc: daniel@yesql.se, pgsql-hackers@postgresql.org
Subject: Re: Possible replace of strncpy on xactdesc.c
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: 
 <CAFsReFVg--+gp8T_QzKos6aUPZRfOupBz=d=1sHg2wc8KkO_WQ@mail.gmail.com>
References: <2F655280-ACEB-47C8-9F0A-35709B96373F@yesql.se>
	<CAFsReFWd5dmy16S5Umsn=peurgpiph0fd4H1BmLtbrMzC_OvPA@mail.gmail.com>
	<CAFsReFVg--+gp8T_QzKos6aUPZRfOupBz=d=1sHg2wc8KkO_WQ@mail.gmail.com>
User-Agent: Mew version 6.8 on Emacs 29.4
Mime-Version: 1.0
Content-Type: Text/Plain; charset=iso-8859-1
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/20260709.151540.712205027352253675.horikyota.ntt%40gmail.com>
Precedence: bulk

Hello,

At Mon, 6 Jul 2026 18:31:44 -0400, Mario Gonz=E1lez Troncoso <gonzalema=
rio@gmail.com> wrote in =

> On Mon, 6 Jul 2026 at 18:26, Mario Gonz=E1lez Troncoso
> <gonzalemario@gmail.com> wrote:
> >
> > On Mon, 6 Jul 2026 at 10:38, Daniel Gustafsson <daniel@yesql.se> wr=
ote:
> > >
> > >
> > > As a general rule it's a good idea to replace strncpy with strlcp=
y.
> > >
> > > > The other functions that are inside the file already use strlcp=
y() so
> > > > maybe the use of current strncpy() on xactdesc.c is just code t=
hat
> > > > comes from the refactor itself.
> > >
> > > It was introduced in 1eb6d6527aae in twophase.c and then moved to=
 xaxtdesc.c in
> > > the above mentioned commit.
> > >
> > > > I'll send a proper patch once some feedback is received but at =
least
> > > > it's compiling and passing local tests.
> > >
> > > Sounds good, please send a patch.
> > >
> >
> > Great. Sending it now after rebasing from master and passing local
> > tests (long live cirrus CI).
> >
> > I added this to the commitfest as well
> > https://commitfest.postgresql.org/patch/6989/

I agree that replacing strncpy() with strlcpy() is often a good general=

direction, but I'm not sure this case fits that pattern.

Here, xlrec->gidlen is the length of the GID including the terminating
NUL byte, and the following pointer advance is based on the same length=
.=

So this looks more like copying a known-length field from the WAL recor=
d
than copying an arbitrary C string.

Wouldn't memcpy(parsed->twophase_gid, bufptr, xlrec->gidlen) express th=
e
intent more directly?

Regards,

-- =

Kyotaro Horiguchi
NTT Open Source Software Center