Re: [PATCH] - Provide robust alternatives for replace_string

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.

> 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);
                size_t          pos = ptr - string->data;

                string->len = pos;

 
> If you're asking for opinion, mine is that StringInfo looks to be the
> better approach, and also you don't need to keep API compatibility.
> 

Thank you.  We also prefer StringInfo solution.

Asim