Hello Michaël,
v23 attached, which does not change the message but does the other fixes.
> + if (coerceToInt(&lval) == INT64_MIN && coerceToInt(&rval) == -1)
> + {
> + fprintf(stderr, "cannot divide INT64_MIN by -1\n");
> + return false;
> + }
> Bike-shedding: "bigint out of range" to match what is done in the backend?
ISTM that it is clearer for the user to say that the issue is with the
division? Otherwise the message does not help much. Well, not that it
would be printed often...
> +/*
> + * Recursive evaluation of an expression in a pgbench script
> + * using the current state of variables.
> + * Returns whether the evaluation was ok,
> + * the value itself is returned through the retval pointer.
> + */
> Could you reformat this comment?
I can.
> fprintf(stderr,
> - "exponential parameter must be
> greater than zero (not \"%s\")\n",
> - argv[5]);
> + "exponential parameter must be greater than
> zero (not \"%s\")\n",
> + argv[5]);
> This is some unnecessary diff noise.
Indeed.
> + setIntValue(retval, getGaussianRand(thread, arg1, arg2,
> dparam));
> Some portions of the code are using tabs instead of spaces between
> function arguments.
Indeed.
> I would as well suggest fixing first the (INT64_MAX / -1) crash on HEAD
> and back-branches with something like the patch attached, inspired from
> int8.c.
I think it is overkill, but do as you feel.
Note that it must also handle modulo, but the code you suggest cannot be
used for that.
#include <stdint.h> int main(int argc, char* argv[]) { int64_t l = INT64_MIN; int64_t r = -1; int64_t d =
l% r; return 0; } // => Floating point exception (core dumped)
ISTM that the second condition can be simplified, as there is no possible
issue if lval is positive?
if (lval < 0 && *resval < 0) { ... }
--
Fabien.
