Horton

Re: Improving psql's \password command

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: "Bossart, Nathan" <bossartn@amazon.com>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2021-11-15T18:12:44Z
Lists: pgsql-hackers 
"Bossart, Nathan" <bossartn@amazon.com> writes:
> Here is an attempt at adding control-C support for \password.  With
> this patch, we pass sigint_interrupt_jmp and sigint_interrupt_enabled
> all the way down to pg_get_line_append(), which is admittedly a bit
> more complicated than I think would be ideal.  I see a couple of other
> calls to simple_prompt() (e.g., \prompt and \connect), and I think the
> same infrastructure could be used for those as well.  I'll send some
> follow-up patches for those if this approach seems reasonable.

Hm.  It's not as bad as I thought it might be, but I still dislike
importing <setjmp.h> into common/string.h --- that seems like a mighty
ugly dependency to have there.  I guess one idea to avoid that is to
declare SigintInterruptContext.jmpbuf as "void *".  Alternatively we
could push those function declarations into some specialized header.

Some other random observations (not a full review):

* API spec for SigintInterruptContext needs to be a bit more detailed.
Maybe "... via an existing SIGINT signal handler that will longjmp to
the specified place, but only when *enabled is true".

* I don't believe that this bit is necessary, or if it is,
the comment fails to justify it:

-	initStringInfo(&buf);
+	/* make sure buf is palloc'd so we don't lose changes after a longjmp */
+	buf = makeStringInfo();

* You're failing to re-enable sigint_ctx->enabled when looping
around for another fgets call.

* Personally I'd write those assignments like

	*(sigint_ctx->enabled) = true;

as that seems clearer.

			regards, tom lane

Commits

  1. pg_receivewal, pg_recvlogical: allow canceling initial password prompt.

  2. Allow psql's other uses of simple_prompt() to be interrupted by ^C.

  3. Provide a variant of simple_prompt() that can be interrupted by ^C.

  4. Make psql's \password default to CURRENT_USER, not PQuser(conn).