On 2020-Aug-03, Asim Praveen wrote:
> 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.
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
