Re: Some doubious code in pgstat.c

At Wed, 4 Nov 2020 22:49:57 +0900, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote in
> On Wed, Nov 4, 2020 at 6:49 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Wed, Nov 4, 2020 at 2:25 PM Kyotaro Horiguchi
> > <horikyota(dot)ntt(at)gmail(dot)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?

Absolutely. the same is seen at several places. Please find the
attached.

As another issue, just replace memcpy with strlcpy makes compiler
complain of type mismatch, as the first paramter to memcpy had an
needless "&" operator. I removed it in this patch.

(&msg.m_slotname is a "char (*)[NAMEDATALEN]", not a "char *".)

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center