Remove arbitrary line length limits in pg_regress (plain and ECPG).

Refactor replace_string() to use a StringInfo for the modifiable
string argument.  This allows the string to be of indefinite size
initially and/or grow substantially during replacement.  The previous
logic in convert_sourcefiles_in() had a hard-wired limit of 1024
bytes on any line in input/*.sql or output/*.out files.  While we've
not had reports of trouble yet, it'd surely have bit us someday.

This also fixes replace_string() so it won't get into an infinite
loop if the string-to-be-replaced is a substring of the replacement.
That's unlikely to happen in current usage, but the function surely
shouldn't depend on it.

Also fix ecpg_filter() to use a StringInfo and thereby remove its
hard limit of 300 bytes on the length of an ecpg source line.

Asim Rama Praveen and Georgios Kokolatos,
reviewed by Alvaro Herrera and myself

Discussion: https://postgr.es/m/y9Dlk2QhiZ39DhaB1QE9mgZ95HcOQKZCNtGwN7XCRKMdBRBnX_0woaRUtTjloEp4PKA6ERmcUcfq3lPGfKPOJ5xX2TV-5WoRYyySeNHRzdw=@protonmail.com

diff --git a/src/interfaces/ecpg/test/pg_regress_ecpg.c b/src/interfaces/ecpg/test/pg_regress_ecpg.c
index 46b9e78fe59d4c582e6ea95aa3d7c5fadd430996..a2d7b70d9a3074f2db64348faf47bb0659dc1724 100644 (file)
--- a/src/interfaces/ecpg/test/pg_regress_ecpg.c
+++ b/src/interfaces/ecpg/test/pg_regress_ecpg.c
@@ -19,8 +19,9 @@
 #include "postgres_fe.h"
 
 #include "pg_regress.h"
+#include "common/string.h"
+#include "lib/stringinfo.h"
 
-#define LINEBUFSIZE 300
 
 static void
 ecpg_filter(const char *sourcefile, const char *outfile)
@@ -31,7 +32,7 @@ ecpg_filter(const char *sourcefile, const char *outfile)
     */
    FILE       *s,
               *t;
-   char        linebuf[LINEBUFSIZE];
+   StringInfoData linebuf;
 
    s = fopen(sourcefile, "r");
    if (!s)
@@ -46,13 +47,14 @@ ecpg_filter(const char *sourcefile, const char *outfile)
        exit(2);
    }
 
-   while (fgets(linebuf, LINEBUFSIZE, s))
+   initStringInfo(&linebuf);
+
+   while (pg_get_line_append(s, &linebuf))
    {
        /* check for "#line " in the beginning */
-       if (strstr(linebuf, "#line ") == linebuf)
+       if (strstr(linebuf.data, "#line ") == linebuf.data)
        {
-           char       *p = strchr(linebuf, '"');
-           char       *n;
+           char       *p = strchr(linebuf.data, '"');
            int         plen = 1;
 
            while (*p && (*(p + plen) == '.' || strchr(p + plen, '/') != NULL))
@@ -62,13 +64,15 @@ ecpg_filter(const char *sourcefile, const char *outfile)
            /* plen is one more than the number of . and / characters */
            if (plen > 1)
            {
-               n = (char *) malloc(plen);
-               strlcpy(n, p + 1, plen);
-               replace_string(linebuf, n, "");
+               memmove(p + 1, p + plen, strlen(p + plen) + 1);
+               /* we don't bother to fix up linebuf.len */
            }
        }
-       fputs(linebuf, t);
+       fputs(linebuf.data, t);
+       resetStringInfo(&linebuf);
    }
+
+   pfree(linebuf.data);
    fclose(s);
    fclose(t);
 }
@@ -87,40 +91,42 @@ ecpg_start_test(const char *testname,
    PID_TYPE    pid;
    char        inprg[MAXPGPATH];
    char        insource[MAXPGPATH];
-   char       *outfile_stdout,
+   StringInfoData testname_dash;
+   char        outfile_stdout[MAXPGPATH],
                expectfile_stdout[MAXPGPATH];
-   char       *outfile_stderr,
+   char        outfile_stderr[MAXPGPATH],
                expectfile_stderr[MAXPGPATH];
-   char       *outfile_source,
+   char        outfile_source[MAXPGPATH],
                expectfile_source[MAXPGPATH];
    char        cmd[MAXPGPATH * 3];
-   char       *testname_dash;
    char       *appnameenv;
 
    snprintf(inprg, sizeof(inprg), "%s/%s", inputdir, testname);
+   snprintf(insource, sizeof(insource), "%s.c", testname);
+
+   initStringInfo(&testname_dash);
+   appendStringInfoString(&testname_dash, testname);
+   replace_string(&testname_dash, "/", "-");
 
-   testname_dash = strdup(testname);
-   replace_string(testname_dash, "/", "-");
    snprintf(expectfile_stdout, sizeof(expectfile_stdout),
             "%s/expected/%s.stdout",
-            outputdir, testname_dash);
+            outputdir, testname_dash.data);
    snprintf(expectfile_stderr, sizeof(expectfile_stderr),
             "%s/expected/%s.stderr",
-            outputdir, testname_dash);
+            outputdir, testname_dash.data);
    snprintf(expectfile_source, sizeof(expectfile_source),
             "%s/expected/%s.c",
-            outputdir, testname_dash);
-
-   /*
-    * We can use replace_string() here because the replacement string does
-    * not occupy more space than the replaced one.
-    */
-   outfile_stdout = strdup(expectfile_stdout);
-   replace_string(outfile_stdout, "/expected/", "/results/");
-   outfile_stderr = strdup(expectfile_stderr);
-   replace_string(outfile_stderr, "/expected/", "/results/");
-   outfile_source = strdup(expectfile_source);
-   replace_string(outfile_source, "/expected/", "/results/");
+            outputdir, testname_dash.data);
+
+   snprintf(outfile_stdout, sizeof(outfile_stdout),
+            "%s/results/%s.stdout",
+            outputdir, testname_dash.data);
+   snprintf(outfile_stderr, sizeof(outfile_stderr),
+            "%s/results/%s.stderr",
+            outputdir, testname_dash.data);
+   snprintf(outfile_source, sizeof(outfile_source),
+            "%s/results/%s.c",
+            outputdir, testname_dash.data);
 
    add_stringlist_item(resultfiles, outfile_stdout);
    add_stringlist_item(expectfiles, expectfile_stdout);
@@ -134,18 +140,15 @@ ecpg_start_test(const char *testname,
    add_stringlist_item(expectfiles, expectfile_source);
    add_stringlist_item(tags, "source");
 
-   snprintf(insource, sizeof(insource), "%s.c", testname);
    ecpg_filter(insource, outfile_source);
 
-   snprintf(inprg, sizeof(inprg), "%s/%s", inputdir, testname);
-
    snprintf(cmd, sizeof(cmd),
             "\"%s\" >\"%s\" 2>\"%s\"",
             inprg,
             outfile_stdout,
             outfile_stderr);
 
-   appnameenv = psprintf("PGAPPNAME=ecpg/%s", testname_dash);
+   appnameenv = psprintf("PGAPPNAME=ecpg/%s", testname_dash.data);
    putenv(appnameenv);
 
    pid = spawn_process(cmd);
@@ -160,10 +163,7 @@ ecpg_start_test(const char *testname,
    unsetenv("PGAPPNAME");
    free(appnameenv);
 
-   free(testname_dash);
-   free(outfile_stdout);
-   free(outfile_stderr);
-   free(outfile_source);
+   free(testname_dash.data);
 
    return pid;
 }

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index d82e0189dcfb236fb2081397b13c78614646c915..74fd026856eab0a70425b1d95e8bd0d282bef240 100644 (file)
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -31,8 +31,10 @@
 
 #include "common/logging.h"
 #include "common/restricted_token.h"
+#include "common/string.h"
 #include "common/username.h"
 #include "getopt_long.h"
+#include "lib/stringinfo.h"
 #include "libpq/pqcomm.h"      /* needed for UNIXSOCK_PATH() */
 #include "pg_config_paths.h"
 #include "pg_regress.h"
@@ -435,22 +437,32 @@ string_matches_pattern(const char *str, const char *pattern)
 }
 
 /*
- * Replace all occurrences of a string in a string with a different string.
- * NOTE: Assumes there is enough room in the target buffer!
+ * Replace all occurrences of "replace" in "string" with "replacement".
+ * The StringInfo will be suitably enlarged if necessary.
+ *
+ * Note: this is optimized on the assumption that most calls will find
+ * no more than one occurrence of "replace", and quite likely none.
  */
 void
-replace_string(char *string, const char *replace, const char *replacement)
+replace_string(StringInfo string, const char *replace, const char *replacement)
 {
+   int         pos = 0;
    char       *ptr;
 
-   while ((ptr = strstr(string, replace)) != NULL)
+   while ((ptr = strstr(string->data + pos, replace)) != NULL)
    {
-       char       *dup = pg_strdup(string);
+       /* Must copy the remainder of the string out of the StringInfo */
+       char       *suffix = pg_strdup(ptr + strlen(replace));
 
-       strlcpy(string, dup, ptr - string + 1);
-       strcat(string, replacement);
-       strcat(string, dup + (ptr - string) + strlen(replace));
-       free(dup);
+       /* Truncate StringInfo at start of found string ... */
+       string->len = ptr - string->data;
+       /* ... and append the replacement (this restores the trailing '\0') */
+       appendStringInfoString(string, replacement);
+       /* Next search should start after the replacement */
+       pos = string->len;
+       /* Put back the remainder of the string */
+       appendStringInfoString(string, suffix);
+       free(suffix);
    }
 }
 
@@ -521,7 +533,7 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
        char        prefix[MAXPGPATH];
        FILE       *infile,
                   *outfile;
-       char        line[1024];
+       StringInfoData line;
 
        /* reject filenames not finishing in ".source" */
        if (strlen(*name) < 8)
@@ -551,15 +563,21 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
                    progname, destfile, strerror(errno));
            exit(2);
        }
-       while (fgets(line, sizeof(line), infile))
+
+       initStringInfo(&line);
+
+       while (pg_get_line_append(infile, &line))
        {
-           replace_string(line, "@abs_srcdir@", inputdir);
-           replace_string(line, "@abs_builddir@", outputdir);
-           replace_string(line, "@testtablespace@", testtablespace);
-           replace_string(line, "@libdir@", dlpath);
-           replace_string(line, "@DLSUFFIX@", DLSUFFIX);
-           fputs(line, outfile);
+           replace_string(&line, "@abs_srcdir@", inputdir);
+           replace_string(&line, "@abs_builddir@", outputdir);
+           replace_string(&line, "@testtablespace@", testtablespace);
+           replace_string(&line, "@libdir@", dlpath);
+           replace_string(&line, "@DLSUFFIX@", DLSUFFIX);
+           fputs(line.data, outfile);
+           resetStringInfo(&line);
        }
+
+       pfree(line.data);
        fclose(infile);
        fclose(outfile);
    }

diff --git a/src/test/regress/pg_regress.h b/src/test/regress/pg_regress.h
index ee6e0d42f401e4798a9daa863141455600df8c9d..726f9c904898d3593bf92271344597592918146c 100644 (file)
--- a/src/test/regress/pg_regress.h
+++ b/src/test/regress/pg_regress.h
@@ -18,6 +18,8 @@
 #define INVALID_PID INVALID_HANDLE_VALUE
 #endif
 
+struct StringInfoData;         /* avoid including stringinfo.h here */
+
 /* simple list of strings */
 typedef struct _stringlist
 {
@@ -49,5 +51,6 @@ int           regression_main(int argc, char *argv[],
                            init_function ifunc, test_function tfunc);
 void       add_stringlist_item(_stringlist **listhead, const char *str);
 PID_TYPE   spawn_process(const char *cmdline);
-void       replace_string(char *string, const char *replace, const char *replacement);
+void       replace_string(struct StringInfoData *string,
+                          const char *replace, const char *replacement);
 bool       file_exists(const char *file);