From d371efb39c33f79ad5f6741d76bfae54df21eb55 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 15 Aug 2018 16:29:32 -0400 Subject: Clean up assorted misuses of snprintf()'s result value. Fix a small number of places that were testing the result of snprintf() but doing so incorrectly. The right test for buffer overrun, per C99, is "result >= bufsize" not "result > bufsize". Some places were also checking for failure with "result == -1", but the standard only says that a negative value is delivered on failure. (Note that this only makes these places correct if snprintf() delivers C99-compliant results. But at least now these places are consistent with all the other places where we assume that.) Also, make psql_start_test() and isolation_start_test() check for buffer overrun while constructing their shell commands. There seems like a higher risk of overrun, with more severe consequences, here than there is for the individual file paths that are made elsewhere in the same functions, so this seemed like a worthwhile change. Also fix guc.c's do_serialize() to initialize errno = 0 before calling vsnprintf. In principle, this should be unnecessary because vsnprintf should have set errno if it returns a failure indication ... but the other two places this coding pattern is cribbed from don't assume that, so let's be consistent. These errors are all very old, so back-patch as appropriate. I think that only the shell command overrun cases are even theoretically reachable in practice, but there's not much point in erroneous error checks. Discussion: https://postgr.es/m/17245.1534289329@sss.pgh.pa.us --- src/test/isolation/isolation_main.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) (limited to 'src/test/isolation/isolation_main.c') diff --git a/src/test/isolation/isolation_main.c b/src/test/isolation/isolation_main.c index dc801b95e19..6ff54737efc 100644 --- a/src/test/isolation/isolation_main.c +++ b/src/test/isolation/isolation_main.c @@ -73,15 +73,27 @@ isolation_start_test(const char *testname, add_stringlist_item(expectfiles, expectfile); if (launcher) + { offset += snprintf(psql_cmd + offset, sizeof(psql_cmd) - offset, "%s ", launcher); + if (offset >= sizeof(psql_cmd)) + { + fprintf(stderr, _("command too long\n")); + exit(2); + } + } - snprintf(psql_cmd + offset, sizeof(psql_cmd) - offset, - "\"%s\" \"dbname=%s\" < \"%s\" > \"%s\" 2>&1", - isolation_exec, - dblist->str, - infile, - outfile); + offset += snprintf(psql_cmd + offset, sizeof(psql_cmd) - offset, + "\"%s\" \"dbname=%s\" < \"%s\" > \"%s\" 2>&1", + isolation_exec, + dblist->str, + infile, + outfile); + if (offset >= sizeof(psql_cmd)) + { + fprintf(stderr, _("command too long\n")); + exit(2); + } pid = spawn_process(psql_cmd); -- cgit v1.2.3