On 12/06/2026 04:53, Ewan Young wrote:
On Thu, Jun 11, 2026 at 11:10 PM <[email protected]> wrote:
The patch attached
Thanks for the report and patch.
I looked at the data flow and agree this is a false positive. startpos is
assigned at the ftell() call inside the first if (difffile) block, and the
only way to reach the use at fseek() with difffile != NULL is for that same
block to have executed (since difffile is reassigned by the fopen(..., "r")
at the end of it).
I should note I couldn't reproduce the warning here: with gcc 12.2 it stays
silent at every optimization level, including -Og.
The fix looks correct to me. The raw diff looks large, but it's almost entirely
re-indentation — git diff -w shows the actual change is just moving the
braces to nest the read block inside the first one.
For the record, the minimal alternative would be long startpos = 0; at the
declaration, which is a one-line diff. I'd lean toward your version, but
I'll leave the final call to a committer.
LGTM as far as the patch goes.
The function feels too nonchalant about errors while opening the file.
The diff output file was created earlier in open_result_files() already,
so it really should exist. I propose the attached, to bail out if the
fopen() fails. (As the original patch, this looks a lot smaller with
"diff -w")
- Heikki
From a3834128019f51a0aa9817f5bdc1303fcaec02c0 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Mon, 15 Jun 2026 13:24:52 +0300
Subject: [PATCH v2] Silence uninitialized variable warning with some compiler
versions
The first "if (difffile)" block initializes the startpos variable, and
the second "if (difffile)" block reads it. The second if-condition can
only be true when the first one was true, so the startpos variable is
always initialized when it's used. However, the compiler might not be
able to deduce that, and warn about startpos being used uninitialized.
To silence the warning, rearrange the if-checks. Also, bail out if the
diff file cannot be opened, instead of ignoring it silently.
Author: Mikhail Litsarev <[email protected]>
Reviewed-by; Ewan Young <[email protected]>
Discussion: https://www.postgresql.org/message-id/[email protected]
---
src/test/regress/pg_regress.c | 46 ++++++++++++++++++-----------------
1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 1c052cc0fbf..b53821dd10c 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1534,31 +1534,33 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul
*/
difffile = fopen(difffilename, "a");
- if (difffile)
- {
- startpos = ftell(difffile);
-
- /* Write diff header */
- fprintf(difffile,
- "diff %s %s %s\n",
- pretty_diff_opts, best_expect_file, resultsfile);
- fclose(difffile);
+ if (!difffile)
+ bail("could not open file \"%s\" for writing: %m", difffilename);
+ startpos = ftell(difffile);
- /* Run diff */
- snprintf(cmd, sizeof(cmd),
- "diff %s \"%s\" \"%s\" >> \"%s\"",
- pretty_diff_opts, best_expect_file, resultsfile, difffilename);
- run_diff(cmd, difffilename);
+ /* Write diff header */
+ fprintf(difffile,
+ "diff %s %s %s\n",
+ pretty_diff_opts, best_expect_file, resultsfile);
+ fclose(difffile);
- /*
- * Reopen the file for reading to emit the diff as TAP diagnostics. We
- * can't keep the file open while diff appends to it, because on
- * Windows the file lock prevents diff from writing.
- */
- difffile = fopen(difffilename, "r");
- }
+ /* Run diff */
+ snprintf(cmd, sizeof(cmd),
+ "diff %s \"%s\" \"%s\" >> \"%s\"",
+ pretty_diff_opts, best_expect_file, resultsfile, difffilename);
+ run_diff(cmd, difffilename);
- if (difffile)
+ /*
+ * Emit the diff output as TAP diagnostics
+ *
+ * Reopen the file for reading. We can't keep the file open while diff
+ * appends to it, because on Windows the file lock prevents diff from
+ * writing.
+ */
+ difffile = fopen(difffilename, "r");
+ if (!difffile)
+ bail("could not open file \"%s\" for reading: %m", difffilename);
+ else
{
/*
* In case of a crash the diff can be huge and all of the subsequent
--
2.47.3
