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
This commit is contained in:
Tom Lane 2020-09-22 15:55:13 -04:00
parent c4133ec169
commit 931487018c
5 changed files with 32 additions and 8 deletions

View File

@ -486,7 +486,7 @@ readfile(const char *path)
result = (char **) pg_malloc(maxlines * sizeof(char *)); result = (char **) pg_malloc(maxlines * sizeof(char *));
n = 0; 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 */ /* make sure there will be room for a trailing NULL pointer */
if (n >= maxlines - 1) if (n >= maxlines - 1)
@ -496,8 +496,6 @@ readfile(const char *path)
} }
result[n++] = pg_strdup(line.data); result[n++] = pg_strdup(line.data);
resetStringInfo(&line);
} }
result[n] = NULL; result[n] = NULL;

View File

@ -45,7 +45,8 @@
* Also note that the palloc'd buffer is usually a lot longer than * Also note that the palloc'd buffer is usually a lot longer than
* strictly necessary, so it may be inadvisable to use this function * strictly necessary, so it may be inadvisable to use this function
* to collect lots of long-lived data. A less memory-hungry option * 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 * char *
pg_get_line(FILE *stream) pg_get_line(FILE *stream)
@ -67,11 +68,37 @@ pg_get_line(FILE *stream)
return buf.data; 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() * pg_get_line_append()
* *
* This has similar behavior to pg_get_line(), and thence to fgets(), * This has similar behavior to pg_get_line(), and thence to fgets(),
* except that the collected data is appended to whatever is in *buf. * 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 * Returns true if a line was successfully collected (including the
* case of a non-newline-terminated line at EOF). Returns false if * case of a non-newline-terminated line at EOF). Returns false if

View File

@ -21,6 +21,7 @@ extern int pg_strip_crlf(char *str);
/* functions in src/common/pg_get_line.c */ /* functions in src/common/pg_get_line.c */
extern char *pg_get_line(FILE *stream); 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); extern bool pg_get_line_append(FILE *stream, struct StringInfoData *buf);
/* functions in src/common/sprompt.c */ /* functions in src/common/sprompt.c */

View File

@ -49,7 +49,7 @@ ecpg_filter(const char *sourcefile, const char *outfile)
initStringInfo(&linebuf); initStringInfo(&linebuf);
while (pg_get_line_append(s, &linebuf)) while (pg_get_line_buf(s, &linebuf))
{ {
/* check for "#line " in the beginning */ /* check for "#line " in the beginning */
if (strstr(linebuf.data, "#line ") == linebuf.data) if (strstr(linebuf.data, "#line ") == linebuf.data)
@ -69,7 +69,6 @@ ecpg_filter(const char *sourcefile, const char *outfile)
} }
} }
fputs(linebuf.data, t); fputs(linebuf.data, t);
resetStringInfo(&linebuf);
} }
pfree(linebuf.data); pfree(linebuf.data);

View File

@ -566,7 +566,7 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
initStringInfo(&line); 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_srcdir@", inputdir);
replace_string(&line, "@abs_builddir@", outputdir); 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, "@libdir@", dlpath);
replace_string(&line, "@DLSUFFIX@", DLSUFFIX); replace_string(&line, "@DLSUFFIX@", DLSUFFIX);
fputs(line.data, outfile); fputs(line.data, outfile);
resetStringInfo(&line);
} }
pfree(line.data); pfree(line.data);