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.