On Thu, Sep 26, 2019 at 10:06:27AM -0300, Alvaro Herrera wrote:
> Hmm, you have an XXX comment that appears to need addressing; and I'd
> add an explanation about the looping behavior to the comment atop the
> new function.
>
> I see that vacuumlo and scripts/common retain their "have_password"
> variables. That seems to be so that they can reuse one for other
> databases. But how does that work with the new routine?
That's the second bullet point I mentioned at the top of the thread
(https://www.postgresql.org/message-id/20190822074558.GG1683@paquier.xyz),
and something I wanted to discuss for this patch:
"Some code paths spawn a password prompt before the first connection
attempt..."
Historically, vacuumlo, scripts/common.c and pg_backup_db.c ask for a
password before doing the first connection attempt, which is different
than any other code paths. That's the reason why have_password is
still there in the case of the first one. scripts/common.c and
pg_dump are different issues as we don't want to repeat the password
for multiple database connections. That point is also related to the
XXX comment, because if we make the logic more consistent with the
rest (aka ask for the password the first time after the first
connection attempt), then we could remove completely the extra
handling with saved_password (meaning no more XXX). That would be
also nicer as we want to keep a fixed size for the password buffer of
100 bytes.
Thinking more about it, keeping connect_with_password_prompt() a
maximum simple would be nicer for future maintenance, and it looks
that we may actually be able to remove allow_password_reuse from
connectDatabase() in common.c, but this needs a rather close lookup as
we should not create any password exposure hazards.
For now I am switching the patch as returned with feedback as there is
much more to consider. And it did not attract much attention either.
--
Michael