Re: Range checks of pg_test_fsync --secs-per-test and pg_test_timing --duration

On 2020-09-10 09:59, Michael Paquier wrote:
>> I notice that the error checking you introduce is different from the checks
>> for pgbench -t and -T (the latter having no errno checks). I'm not sure
>> which is correct, but it's perhaps worth making them the same.
> pgbench currently uses atoi() to parse the options of -t and -T. Are
> you suggesting to switch that to strtoXX() as well or perhaps you are
> referring to the parsing of the weight in parseScriptWeight()? FWIW,
> the error handling introduced in this patch is similar to what we do
> for example in pg_resetwal. This has its own problems as strtoul()
> would not report ERANGE except for values higher than ULONG_MAX, but
> the returned results are stored in 32 bits. We could switch to just
> use uint64 where we could of course, but is that really worth it for
> such tools? For example, pg_test_timing could overflow the
> total_timing calculated if using a too high value, but nobody would
> use such values anyway. So I'd rather just use uint32 and call it a
> day, for simplicity's sake mainly..

The first patch you proposed checks for errno == ERANGE, but pgbench
code doesn't do that. So one of them is not correct.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services