At Thu, 5 Nov 2020 11:48:24 +0530, Amit Kapila <amit.kapil...@gmail.com> wrote in > On Thu, Nov 5, 2020 at 9:44 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > On Thu, Nov 5, 2020 at 11:18 AM Kyotaro Horiguchi > > <horikyota....@gmail.com> wrote: > > > 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 *".) > > > > > > > The patch looks good to me. > > > > LGTM as well but the proposed commit message seems to be a bit > unclear. How about something like this: > "Use strlcpy instead of memcpy for copying the slot name in pgstat.c. > > There is no outright bug here but it is better to be consistent with > the usage at other places in the same file. In the passing, fix a wrong > Assertion in pgstat_recv_replslot."
Looks better, thanks. By the way, I noticed the following sequence. pgstat.c: 3204 3204> lbeentry.st_appname[0] = '\0'; 3205> if (MyProcPort && MyProcPort->remote_hostname) 3206> strlcpy(lbeentry.st_clienthostname, MyProcPort->remote_hostname, 3207> NAMEDATALEN); 3208> else 3209> lbeentry.st_clienthostname[0] = '\0'; 3210> lbeentry.st_activity_raw[0] = '\0'; 3211> /* Also make sure the last byte in each string area is always 0 */ 3212> lbeentry.st_appname[NAMEDATALEN - 1] = '\0'; 3213> lbeentry.st_clienthostname[NAMEDATALEN - 1] = '\0'; 3214> lbeentry.st_activity_raw[pgstat_track_activity_query_size - 1] = '\0'; The strlcpy at the line 3206 makes sure that st_clienthostname is null-terminated so it's nonsense to do line 3213. st_appname and st_activity_raw are set to zero-length string. Is there any point in setting terminating nul to them? -- Kyotaro Horiguchi NTT Open Source Software Center
