Re: buildfarm warnings
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: pgsql-hackers@postgresql.org, Robert Haas <rhaas@postgresql.org>,
Alexander Korotkov <akorotkov@postgresql.org>
Date: 2022-02-17T17:08:01Z
Lists: pgsql-hackers
I wrote: > Justin Pryzby <pryzby@telsasoft.com> writes: >> pg_basebackup.c:1261:35: warning: storing the address of local variable archive_filename in progress_filename [-Wdangling-pointer=] >> => new in 23a1c6578 - looks like a real error > I saw that one a few days ago but didn't get around to looking > more closely yet. It does look fishy, but it might be okay > depending on when the global variable can be accessed. I got around to looking at this, and it absolutely is a bug. The test scripts don't reach it, because they don't exercise -P mode, let alone -P -v which is what you need to get progress_report() to try to print the filename. However, if you modify the test to do so, then you find that the output differs depending on whether or not you add "progress_filename = NULL;" at the bottom of CreateBackupStreamer(). So the variable is continuing to be referenced, not only after it goes out of scope within that function, but even after the function returns entirely. (Interestingly, the output isn't obvious garbage, at least not on my machine; so somehow the array's part of the stack is not getting overwritten very soon.) A quick-and-dirty fix is - progress_filename = archive_filename; + progress_filename = pg_strdup(archive_filename); but perhaps this needs more thought. How long is that filename actually reasonable to show in the progress reports? regards, tom lane
Commits
-
Avoid dangling-pointer usage in pg_basebackup progress reports.
- 62cb7427d1e4 15.0 landed
-
Delete contrib/xml2's legacy implementation of xml_is_well_formed().
- fd2abeb7c2b2 15.0 landed