Re: Avoid incomplete copy string (src/backend/access/transam/xlog.c) - Mailing list pgsql-hackers

From Yugo NAGATA
Subject Re: Avoid incomplete copy string (src/backend/access/transam/xlog.c)
Date
Msg-id 20240627121704.57222340fd1ca2c0256c3340@sranhm.sraoss.co.jp
Whole thread Raw
In response to Avoid incomplete copy string (src/backend/access/transam/xlog.c)  (Ranier Vilela <ranier.vf@gmail.com>)
Responses Re: Avoid incomplete copy string (src/backend/access/transam/xlog.c)
List pgsql-hackers
On Mon, 24 Jun 2024 08:37:26 -0300
Ranier Vilela <ranier.vf@gmail.com> wrote:

> Em seg., 24 de jun. de 2024 às 00:27, Yugo NAGATA <nagata@sraoss.co.jp>
> escreveu:
> 
> > On Sun, 23 Jun 2024 22:34:03 -0300
> > Ranier Vilela <ranier.vf@gmail.com> wrote:
> >
> > > Em dom., 23 de jun. de 2024 às 22:14, Ranier Vilela <ranier.vf@gmail.com
> > >
> > > escreveu:
> > >
> > > > Em dom., 23 de jun. de 2024 às 22:05, Ranier Vilela <
> > ranier.vf@gmail.com>
> > > > escreveu:
> > > >
> > > >> Em dom., 23 de jun. de 2024 às 21:54, Michael Paquier <
> > > >> michael@paquier.xyz> escreveu:
> > > >>
> > > >>> On Sun, Jun 23, 2024 at 09:34:45PM -0300, Ranier Vilela wrote:
> > > >>> > It's not critical code, so I think it's ok to use strlen, even
> > because
> > > >>> the
> > > >>> > result of strlen will already be available using modern compilers.
> > > >>> >
> > > >>> > So, I think it's ok to use memcpy with strlen + 1.
> > > >>>
> > > >>> It seems to me that there is a pretty good argument to just use
> > > >>> strlcpy() for the same reason as the one you cite: this is not a
> > > >>> performance-critical code, and that's just safer.
> > > >>>
> > > >> Yeah, I'm fine with strlcpy. I'm not against it.
> > > >>
> > > > Perhaps, like the v2?
> > > >
> > > > Either v1 or v2, to me, looks good.
> > > >
> > > Thinking about, does not make sense the field size MAXPGPATH + 1.
> > > all other similar fields are just MAXPGPATH.
> > >
> > > If we copy MAXPGPATH + 1, it will also be wrong.
> > > So it is necessary to adjust logbackup.h as well.
> >
> > I am not sure whether we need to change the size of the field,
> > but if change it, I wonder it is better to modify the following
> > message from MAXPGPATH to MAXPGPATH -1.
> >
> >                                  errmsg("backup label too long (max %d
> > bytes)",
> >                                                 MAXPGPATH)));
> >
> Or perhaps, is it better to show the too long label?
> 
> errmsg("backup label too long (%s)",
>                                                 backupidstr)));

I don't think it is better to show a string longer than MAXPGPATH (=1024)
in the error message.

Regards,
Yugo Nagata

> 
> best regards,
> Ranier Vilela
> 
> >
> > >
> > > So, I think that v3 is ok to fix.
> > >
> > > best regards,
> > > Ranier Vilela
> > >
> > > >
> > > > best regards,
> > > > Ranier Vilela
> > > >
> > > >>
> >
> >
> > --
> > Yugo NAGATA <nagata@sraoss.co.jp>
> >


-- 
Yugo NAGATA <nagata@sraoss.co.jp>



pgsql-hackers by date:

Previous
From: Yugo NAGATA
Date:
Subject: Re: Document NULL
Next
From: Yugo NAGATA
Date:
Subject: Re: Avoid incomplete copy string (src/backend/access/transam/xlog.c)