Re: Some doubious code in pgstat.c
Masahiko Sawada <sawada.mshk@gmail.com>
From: Masahiko Sawada <sawada.mshk@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2020-11-04T13:49:57Z
Lists: pgsql-hackers
On Wed, Nov 4, 2020 at 6:49 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Wed, Nov 4, 2020 at 2:25 PM Kyotaro Horiguchi > <horikyota.ntt@gmail.com> wrote: > > > > Hello. > > > > While updating a patch, I noticed that the replication slot stats > > patch (9868167500) put some somewhat doubious codes. > > > > In pgstat_recv_replslot, an assertion like the following exists: > > > > > idx = pgstat_replslot_index(msg->m_slotname, !msg->m_drop); > > .. > > > Assert(idx >= 0 && idx < max_replication_slots); > > > > But the idx should be 0..(max_replication_slots - 1). > > > > Right. > > > > > In the same function the following code assumes that the given "char > > *name" has the length of NAMEDATALEN. It actually is, but that > > assumption seems a bit bogus. I think it should use strlcpy instead. > > > > Agreed. +1 The commit uses memcpy in the same way in other places too, for instance in pgstat_report_replslot_drop(). Should we fix all of them? Regards, -- Masahiko Sawada EnterpriseDB: https://www.enterprisedb.com/
Commits
-
Use strlcpy instead of memcpy for copying the slot name in pgstat.c.
- 4f841ce3f7f4 14.0 landed
-
Rearrange pgstat_bestart() to avoid failures within its critical section.
- 85ccb6899c6c 12.0 cited