Rethink API for pg_get_line.c, one more time.
authorTom Lane <[email protected]>
Tue, 22 Sep 2020 19:55:13 +0000 (15:55 -0400)
committerTom Lane <[email protected]>
Tue, 22 Sep 2020 19:55:13 +0000 (15:55 -0400)
Further experience says that the appending behavior offered by
pg_get_line_append is useful to only a very small minority of callers.
For most, the requirement to reset the buffer after each line is just
an error-prone nuisance.  Hence, invent another alternative call
pg_get_line_buf, which takes care of that detail.

Noted while reviewing a patch from Daniel Gustafsson.

Discussion: https://postgr.es/m/48A4FA71-524E-41B9-953A-FD04EF36E2E7@yesql.se

src/bin/initdb/initdb.c
src/common/pg_get_line.c
src/include/common/string.h
src/interfaces/ecpg/test/pg_regress_ecpg.c
src/test/regress/pg_regress.c

index 37e0d7ceab93cd8b10b4806fa006d43ab7046b44..118b282d1c5e21adb022e2b4a612bc76960e0e8d 100644 (file)
@@ -486,7 +486,7 @@ readfile(const char *path)
    result = (char **) pg_malloc(maxlines * sizeof(char *));
 
    n = 0;
-   while (pg_get_line_append(infile, &line))
+   while (pg_get_line_buf(infile, &line))
    {
        /* make sure there will be room for a trailing NULL pointer */
        if (n >= maxlines - 1)
@@ -496,8 +496,6 @@ readfile(const char *path)
        }
 
        result[n++] = pg_strdup(line.data);
-
-       resetStringInfo(&line);
    }
    result[n] = NULL;
 
index 2fb8e198933dfd3d805c9b10a1827191c1a1af4a..9eb1a33bbb3664f4f907866e5c949625143853ce 100644 (file)
@@ -45,7 +45,8 @@
  * Also note that the palloc'd buffer is usually a lot longer than
  * strictly necessary, so it may be inadvisable to use this function
  * to collect lots of long-lived data.  A less memory-hungry option
- * is to use pg_get_line_append() in a loop, then pstrdup() each line.
+ * is to use pg_get_line_buf() or pg_get_line_append() in a loop,
+ * then pstrdup() each line.
  */
 char *
 pg_get_line(FILE *stream)
@@ -67,11 +68,37 @@ pg_get_line(FILE *stream)
    return buf.data;
 }
 
+/*
+ * pg_get_line_buf()
+ *
+ * This has similar behavior to pg_get_line(), and thence to fgets(),
+ * except that the collected data is returned in a caller-supplied
+ * StringInfo buffer.  This is a convenient API for code that just
+ * wants to read and process one line at a time, without any artificial
+ * limit on line length.
+ *
+ * Returns true if a line was successfully collected (including the
+ * case of a non-newline-terminated line at EOF).  Returns false if
+ * there was an I/O error or no data was available before EOF.
+ * (Check ferror(stream) to distinguish these cases.)
+ *
+ * In the false-result case, buf is reset to empty.
+ */
+bool
+pg_get_line_buf(FILE *stream, StringInfo buf)
+{
+   /* We just need to drop any data from the previous call */
+   resetStringInfo(buf);
+   return pg_get_line_append(stream, buf);
+}
+
 /*
  * pg_get_line_append()
  *
  * This has similar behavior to pg_get_line(), and thence to fgets(),
  * except that the collected data is appended to whatever is in *buf.
+ * This is useful in preference to pg_get_line_buf() if the caller wants
+ * to merge some lines together, e.g. to implement backslash continuation.
  *
  * Returns true if a line was successfully collected (including the
  * case of a non-newline-terminated line at EOF).  Returns false if
index 50c241a811b6a149ac4070d1cfb4503575883a70..6a4baa6f3590031b3c6a2240fb62c5f705a90639 100644 (file)
@@ -21,6 +21,7 @@ extern int    pg_strip_crlf(char *str);
 
 /* functions in src/common/pg_get_line.c */
 extern char *pg_get_line(FILE *stream);
+extern bool pg_get_line_buf(FILE *stream, struct StringInfoData *buf);
 extern bool pg_get_line_append(FILE *stream, struct StringInfoData *buf);
 
 /* functions in src/common/sprompt.c */
index a2d7b70d9a3074f2db64348faf47bb0659dc1724..6e1d25b1f4a3cddb484eedb752da0d4d42cc7004 100644 (file)
@@ -49,7 +49,7 @@ ecpg_filter(const char *sourcefile, const char *outfile)
 
    initStringInfo(&linebuf);
 
-   while (pg_get_line_append(s, &linebuf))
+   while (pg_get_line_buf(s, &linebuf))
    {
        /* check for "#line " in the beginning */
        if (strstr(linebuf.data, "#line ") == linebuf.data)
@@ -69,7 +69,6 @@ ecpg_filter(const char *sourcefile, const char *outfile)
            }
        }
        fputs(linebuf.data, t);
-       resetStringInfo(&linebuf);
    }
 
    pfree(linebuf.data);
index 74fd026856eab0a70425b1d95e8bd0d282bef240..23d7d0beb2e8dc261e2b9aa802761d3366a8a64f 100644 (file)
@@ -566,7 +566,7 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
        initStringInfo(&line);
 
-       while (pg_get_line_append(infile, &line))
+       while (pg_get_line_buf(infile, &line))
        {
            replace_string(&line, "@abs_srcdir@", inputdir);
            replace_string(&line, "@abs_builddir@", outputdir);
@@ -574,7 +574,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
            replace_string(&line, "@libdir@", dlpath);
            replace_string(&line, "@DLSUFFIX@", DLSUFFIX);
            fputs(line.data, outfile);
-           resetStringInfo(&line);
        }
 
        pfree(line.data);