Re: Remove unnecessary code from psql's watch command

On Wed, 06 Mar 2024 13:03:39 -0500
Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Michael Paquier <michael(at)paquier(dot)xyz> writes:
> > On Tue, Mar 05, 2024 at 10:05:52PM +0900, Yugo NAGATA wrote:
> >> In the current code of do_watch(), sigsetjmp is called if WIN32
> >> is defined, but siglongjmp is not called in the signal handler
> >> in this condition. On Windows, currently, cancellation is checked
> >> only by cancel_pressed, and calling sigsetjmp in do_watch() is
> >> unnecessary. Therefore, we can remove code around sigsetjmp in
> >> do_watch(). I've attached the patch for this fix.
>
> > Re-reading the top comment of sigint_interrupt_enabled, it looks like
> > you're right here. As long as we check for cancel_pressed there
> > should be no need for any special cancellation handling here.
>
> I don't have Windows here to test on, but does the WIN32 code
> path work at all? It looks to me like cancel_pressed becoming
> true doesn't get us to exit the outer loop, only the inner delay
> one, meaning that trying to control-C out of a \watch will just
> cause it to repeat the command even faster. That path isn't
> setting the "done" variable, and it isn't checking it either,
> because all of that only happens in the other #ifdef arm.

The outer loop is eventually exited even because PSQLexecWatch returns 0
when cancel_pressed = 0. However, it happens after executing an extra
query in this function not just after exit of the inner loop. Therefore,
it would be better to adding set and check of "done" in WIN32, too.

I've attached the updated patch (v2_remove_unnecessary_code_in_psql_watch.patch).

Regards,
Yugo Nagata

--
Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>