Re: psql's \watch is broken

Hello Tom,

My 0.02 €:

> Given the rather small number of existing uses of CancelRequested,
> I wonder if it wouldn't be a better idea to rename it to cancel_pressed?

I prefer the former because it is more functional (a cancellation has been
requested, however the mean to do so) while "pressed" rather suggest a
particular operation.

> Also, perhaps I am missing something, but I do not see anyplace in the
> current code base that ever *clears* CancelRequested.

This was already like that in the initial version before the refactoring.

./src/bin/scripts/common.h:extern bool CancelRequested;
./src/bin/scripts/common.c:bool CancelRequested = false;
./src/bin/scripts/common.c: CancelRequested = true;
./src/bin/scripts/common.c: CancelRequested = true;
./src/bin/scripts/common.c: CancelRequested = true;
./src/bin/scripts/common.c: CancelRequested = true;
./src/bin/scripts/vacuumdb.c: if (CancelRequested)
./src/bin/scripts/vacuumdb.c: if (CancelRequested)
./src/bin/scripts/vacuumdb.c: if (i < 0 || CancelRequested)

However "cancel_request" resets are in "psql/mainloop.c", and are changed
to "CancelRequest = false" resets by Michaël patch, so all seems fine.

> How much has this code been tested? Is it really sane to remove the
> setting of that flag from psql_cancel_callback, as this patch does?

ISTM that the callback is called from a function which sets CancelRequest?

> Is it sane that CancelRequested isn't declared volatile?

I agree that it would seem appropriate, but the initial version I
refactored was not declaring CancelRequested as volatile, so I did not
change that. However, "cancel_pressed" is volatile, so merging the two
would indeed suggest to declare it as volatile.

--
Fabien.