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
