On Tue, 4 Aug 2020 at 00:12, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I wrote:
> > David Rowley <dgrowleyml@gmail.com> writes:
> >> Will mean that we'll now no longer zero the full length of the m_xlog
> >> field after the end of the string. Won't that mean we'll start writing
> >> junk bytes to the stats collector?
>
> > StrNCpy doesn't zero-fill the destination today either (except for
> > the very last byte).
>
> Oh, no, I take that back --- didn't read all of the strncpy man
> page :-(. Yeah, this is a point. We'd need to check each call
> site to see whether the zero-padding matters.
I just had a thought that even strlcpy() is not really ideal for many
of our purposes for it.
Currently we still have cruddy code like:
strlcpy(fullname, pg_TZDIR(), sizeof(fullname));
if (strlen(fullname) + 1 + strlen(name) >= MAXPGPATH)
return -1; /* not gonna fit */
strcat(fullname, "/");
strcat(fullname, name);
If strlcpy() had been designed differently to take a signed size and
do nothing when the size is <= 0 then we could have had much cleaner
implementations of the above:
size_t len;
len = strlcpy(fullname, pg_TZDIR(), sizeof(fullname));
len += strlcpy(fullname + len, "/", sizeof(fullname) - len);
len += strlcpy(fullname + len, name, sizeof(fullname) - len);
if (len >= sizeof(fullname))
return -1; /* didn't fit */
This should be much more efficient, in general, due to the lack of
strlen() calls and the concatenation not having to refind the end of
the string again each time.
Now, I'm not saying we should change strlcpy() to take a signed type
instead of size_t to allow this to work. Reusing that name for another
purpose is likely a bad idea that will lead to misuse and confusion.
What I am saying is that perhaps strlcpy() is not all that it's
cracked up to be and it could have been done better. Perhaps we can
invent our own version that fixes the shortcomings.
Just a thought.
On the other hand, perhaps we're not using the return value of
strlcpy() enough for such a change to make sense. However, a quick
glance shows we certainly could use it more often, e.g:
if (parsed->xinfo & XACT_XINFO_HAS_GID)
{
strlcpy(parsed->twophase_gid, data, sizeof(parsed->twophase_gid));
data += strlen(data) + 1;
}
David