Re: Possible replace of strncpy on xactdesc.c

On Thu, 9 Jul 2026 at 02:15, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> Hello,
>
> At Mon, 6 Jul 2026 18:31:44 -0400, Mario González Troncoso <gonzalemario(at)gmail(dot)com> wrote in
> > On Mon, 6 Jul 2026 at 18:26, Mario González Troncoso
> > <gonzalemario(at)gmail(dot)com> wrote:
> > >
> > > On Mon, 6 Jul 2026 at 10:38, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
> > > >
> > > >
> > > > As a general rule it's a good idea to replace strncpy with strlcpy.
> > > >
> > > > > The other functions that are inside the file already use strlcpy() so
> > > > > maybe the use of current strncpy() on xactdesc.c is just code that
> > > > > 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.
> > > >
> > >

Unfortunately ( :-) for me ) the same was commited in ` dd50eb9145e
Use more strlcpy() in two-phase transaction code` whilst this was long
in review

Thanks for the review guys anyway. I think I should've tried to find a
committer sonner. I'll close the entry in the commitfest.
https://commitfest.postgresql.org/patch/6989/

> > > 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 record
> than copying an arbitrary C string.
>
> Wouldn't memcpy(parsed->twophase_gid, bufptr, xlrec->gidlen) express the
> intent more directly?
>
> Regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center

--
Mario Gonzalez