Date: Mon, 3 Aug 2020 11:06:40 -0400
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Asim Praveen <pasim@vmware.com>
Cc: Georgios <gkokolatos@protonmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: [PATCH] - Provide robust alternatives for replace_string
Message-ID: <20200803150640.GA30310@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <F57E6876-C6F0-4A4F-9F3E-F06F07A3803A@vmware.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On 2020-Aug-03, Asim Praveen wrote:

> Thank you Alvaro for reviewing the patch!
> 
> > On 01-Aug-2020, at 7:22 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> > 
> > What happens if a replacement string happens to be split in the middle
> > by the fgets buffering?  I think it'll fail to be replaced.  This
> > applies to both versions.
> 
> Can a string to be replaced be split across multiple lines in the source file?  If I understand correctly, fgets reads one line from input file at a time.  If I do not, in the worst case, we will get an un-replaced string in the output, such as “@abs_dir@“ and it should be easily detected by a failing diff.

I meant what if the line is longer than 1023 chars and the replace
marker starts at byte 1021, for example.  Then the first fgets would get
"@ab" and the second fgets would get "s_dir@" and none would see it as
replaceable.

> > In the stringinfo version it seemed to me that using pnstrdup is
> > possible to avoid copying trailing bytes.
> 
> That’s a good suggestion.  Using pnstrdup would look like this:
> 
> --- a/src/test/regress/pg_regress.c
> +++ b/src/test/regress/pg_regress.c
> @@ -465,7 +465,7 @@ replace_stringInfo(StringInfo string, const char *replace, const char *replaceme
> 
>         while ((ptr = strstr(string->data, replace)) != NULL)
>         {
> -               char       *dup = pg_strdup(string->data);
> +              char       *dup = pnstrdup(string->data, string->maxlen);

I was thinking pnstrdup(string->data, ptr - string->data) to avoid
copying the chars beyond ptr.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services