Date: Mon, 24 Jun 2024 12:27:28 +0900
From: Yugo NAGATA <nagata@sraoss.co.jp>
To: Ranier Vilela <ranier.vf@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, =?UTF-8?B?RmFicsOtemlv?= de Royes
 Mello <fabriziomello@gmail.com>, Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Avoid incomplete copy string
 (src/backend/access/transam/xlog.c)
Message-Id: <20240624122728.95d555f1358547a75abebd85@sranhm.sraoss.co.jp>
In-Reply-To: <CAEudQAp2L2GBMwiH6+OO5ZAnZmq2zdUEAu1=VMpYhdRUWvakEQ@mail.gmail.com.sranhm>
References: <CAEudQApAsbLsQ+gGiw-hT+JwGhgogFa_=5NUkgFO6kOPxyNidQ@mail.gmail.com>
	<CAFcNs+p2NSyT0Vs2gFxGwFn02jEj_DgHWK0N-F18EtENh-8TEw@mail.gmail.com>
	<Zni8lmBGRe_-kcu3@paquier.xyz>
	<CAEudQAoED8DCDxfpGmZzjw+jCEd+0aTvCf=cComZsRjENW05HQ@mail.gmail.com>
	<ZnjDoP_MZQefJfwJ@paquier.xyz>
	<CAEudQAq=bffL+=76T2R5A8mde191XwwxWevj1iU1K83HabTgtA@mail.gmail.com>
	<CAEudQApOsin4JVJG+4WjZnGTV7ikEq+13s0F6PwjtgKWrBgzKw@mail.gmail.com>
	<CAEudQAp2L2GBMwiH6+OO5ZAnZmq2zdUEAu1=VMpYhdRUWvakEQ@mail.gmail.com.sranhm>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/20240624122728.95d555f1358547a75abebd85%40sranhm.sraoss.co.jp>
Precedence: bulk

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

Regards,
Yugo Nagata

> 
> So, I think that v3 is ok to fix.
> 
> best regards,
> Ranier Vilela
> 
> >
> > best regards,
> > Ranier Vilela
> >
> >>


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