"Bossart, Nathan" <bossa...@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