Hello, At Mon, 6 Jul 2026 18:31:44 -0400, Mario González Troncoso <[email protected]> wrote in > On Mon, 6 Jul 2026 at 18:26, Mario González Troncoso > <[email protected]> wrote: > > > > On Mon, 6 Jul 2026 at 10:38, Daniel Gustafsson <[email protected]> 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. > > > > > > > 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
