On Mon, Mar 13, 2023 at 10:17:12AM +0900, Michael Paquier wrote:
> I am not sure that this will be the last option we'll ever add to
> \watch, so I'd rather have us choose a design more flexible than
> what's proposed here, in a way similar to \g or \gx.
While on it, I have some comments about 0001.
- sleep = strtod(opt, NULL);
- if (sleep <= 0)
- sleep = 1;
+ char *opt_end;
+ sleep = strtod(opt, &opt_end);
+ if (sleep < 0 || *opt_end)
+ {
+ pg_log_error("\\watch interval must be non-negative number, "
+ "but argument is '%s'", opt);
+ free(opt);
+ resetPQExpBuffer(query_buf);
+ psql_scan_reset(scan_state);
+ return PSQL_CMD_ERROR;
+ }
Okay by me to make this behavior a bit better, though it is not
something I would backpatch as it can influence existing workflows,
even if they worked in an inappropriate way.
Anyway, are you sure that this is actually OK? It seems to me that
this needs to check for three things:
- If sleep is a negative value.
- errno should be non-zero.
- *opt_end == opt.
So this needs three different error messages to show the exact error
to the user? Wouldn't it be better to have a couple of regression
tests, as well?
--
Michael