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.gg1...@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
signature.asc
Description: PGP signature
