Rethink API for pg_get_line.c, one more time.

tglsfdc · tglsfdc · commit 931487018c40 · 2020-09-22T15:55:13.000-04:00
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
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
@@ -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;
 
diff --git a/src/common/pg_get_line.c b/src/common/pg_get_line.c
@@ -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
diff --git a/src/include/common/string.h b/src/include/common/string.h
@@ -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 */
diff --git a/src/interfaces/ecpg/test/pg_regress_ecpg.c b/src/interfaces/ecpg/test/pg_regress_ecpg.c
@@ -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);
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
@@ -566,15 +566,14 @@ 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);
 			replace_string(&line, "@testtablespace@", testtablespace);
 			replace_string(&line, "@libdir@", dlpath);
 			replace_string(&line, "@DLSUFFIX@", DLSUFFIX);
 			fputs(line.data, outfile);
-			resetStringInfo(&line);
 		}
 
 		pfree(line.data);

Original file line number	Diff line number	Diff line change
`@@ -486,7 +486,7 @@ readfile(const char *path)`
`486`	`486`	`result = (char *) pg_malloc(maxlines sizeof(char *));`
`487`	`487`
`488`	`488`	`n = 0;`
`489`		`- while (pg_get_line_append(infile, &line))`
	`489`	`+ while (pg_get_line_buf(infile, &line))`
`490`	`490`	`{`
`491`	`491`	`/* make sure there will be room for a trailing NULL pointer */`
`492`	`492`	`if (n >= maxlines - 1)`
`@@ -496,8 +496,6 @@ readfile(const char *path)`
`496`	`496`	`}`
`497`	`497`
`498`	`498`	`result[n++] = pg_strdup(line.data);`
`499`		`-`
`500`		`- resetStringInfo(&line);`
`501`	`499`	`}`
`502`	`500`	`result[n] = NULL;`
`503`	`501`
Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,7 @@ ecpg_filter(const char sourcefile, const char outfile)`
`49`	`49`
`50`	`50`	`initStringInfo(&linebuf);`
`51`	`51`
`52`		`- while (pg_get_line_append(s, &linebuf))`
	`52`	`+ while (pg_get_line_buf(s, &linebuf))`
`53`	`53`	`{`
`54`	`54`	`/* check for "#line " in the beginning */`
`55`	`55`	`if (strstr(linebuf.data, "#line ") == linebuf.data)`
`@@ -69,7 +69,6 @@ ecpg_filter(const char sourcefile, const char outfile)`
`69`	`69`	`}`
`70`	`70`	`}`
`71`	`71`	`fputs(linebuf.data, t);`
`72`		`- resetStringInfo(&linebuf);`
`73`	`72`	`}`
`74`	`73`
`75`	`74`	`pfree(linebuf.data);`
Original file line number	Diff line number	Diff line change
`@@ -566,15 +566,14 @@ convert_sourcefiles_in(const char source_subdir, const char dest_dir, const ch`
`566`	`566`
`567`	`567`	`initStringInfo(&line);`
`568`	`568`
`569`		`- while (pg_get_line_append(infile, &line))`
	`569`	`+ while (pg_get_line_buf(infile, &line))`
`570`	`570`	`{`
`571`	`571`	`replace_string(&line, "@abs_srcdir@", inputdir);`
`572`	`572`	`replace_string(&line, "@abs_builddir@", outputdir);`
`573`	`573`	`replace_string(&line, "@testtablespace@", testtablespace);`
`574`	`574`	`replace_string(&line, "@libdir@", dlpath);`
`575`	`575`	`replace_string(&line, "@DLSUFFIX@", DLSUFFIX);`
`576`	`576`	`fputs(line.data, outfile);`
`577`		`- resetStringInfo(&line);`
`578`	`577`	`}`
`579`	`578`
`580`	`579`	`pfree(line.data);`