Subject: Re: Avoid incomplete copy string (src/backend/access/transam/xlog.c)

From: Ranier Vilela <[email-masked]>
Date: Sun, 23 Jun 2024 22:05:42 -0300

Em dom., 23 de jun. de 2024 às 21:54, Michael Paquier <[email-masked]>
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.

New version, attached.

best regards,
Ranier Vilela

---

From: Ranier Vilela <[email-masked]>
Date: Sun, 23 Jun 2024 22:14:01 -0300

> Yeah, I'm fine with strlcpy. I'm not against it.
Perhaps, like the v2?

Either v1 or v2, to me, looks good.

best regards,
Ranier Vilela

---

From: Ranier Vilela <[email-masked]>
Date: Sun, 23 Jun 2024 22:34:03 -0300

Em dom., 23 de jun. de 2024 às 22:14, Ranier Vilela <[email-masked]>
escreveu:

>> Em dom., 23 de jun. de 2024 às 21:54, Michael Paquier <
>> [email-masked]> escreveu:
>>
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.

So, I think that v3 is ok to fix.

best regards,
Ranier Vilela

---

From: Yugo NAGATA <[email-masked]>
Date: Mon, 24 Jun 2024 12:27:28 +0900

On Sun, 23 Jun 2024 22:34:03 -0300

> 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)));

Regards,
Yugo Nagata

-- 
Yugo NAGATA <[email-masked]>

---

From: Yugo NAGATA <[email-masked]>
Date: Thu, 27 Jun 2024 12:17:04 +0900

On Mon, 24 Jun 2024 08:37:26 -0300
Ranier Vilela <[email-masked]> wrote:

> Em seg., 24 de jun. de 2024 às 00:27, Yugo NAGATA <[email-masked]>
> escreveu:
> 
> > > Em dom., 23 de jun. de 2024 às 22:14, Ranier Vilela <[email-masked]
> > >
> > > escreveu:
> > >
> 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

---

From: Michael Paquier <[email-masked]>
Date: Thu, 27 Jun 2024 13:04:22 +0900

> I don't think it is better to show a string longer than MAXPGPATH (=1024)

+1.  I am not convinced that this is useful in this context.
--
Michael