From 931487018c409a3102452f965ccaa48367244a41 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 22 Sep 2020 15:55:13 -0400 Subject: [PATCH] Rethink API for pg_get_line.c, one more time. 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 | 4 +-- src/common/pg_get_line.c | 29 +++++++++++++++++++++- src/include/common/string.h | 1 + src/interfaces/ecpg/test/pg_regress_ecpg.c | 3 +-- src/test/regress/pg_regress.c | 3 +-- 5 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 37e0d7ceab..118b282d1c 100644 --- 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 index 2fb8e19893..9eb1a33bbb 100644 --- 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 index 50c241a811..6a4baa6f35 100644 --- 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 index a2d7b70d9a..6e1d25b1f4 100644 --- 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 index 74fd026856..23d7d0beb2 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -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);