On 12/06/2026 04:53, Ewan Young wrote:
> On Thu, Jun 11, 2026 at 11:10 PM <m.litsarev@postgrespro.ru> 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