Thank you Alvaro for reviewing the patch! > On 01-Aug-2020, at 7:22 AM, Alvaro Herrera <alvhe...@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