On 15.04.21 09:08, Willy Tarreau wrote:
On Wed, Apr 14, 2021 at 09:52:31PM +0200, Aleksandar Lazic wrote:
+ - string : This is the default search type and returns a String;
+ - boolean : If the JSON value is not a String or a Number
+ - number : When the JSON value is a Number then will the value be
+ converted to a String. If its known that the value is a
+ integer then add 'int' to the <output_type> which helps
+ haproxy to convert the value to a integer for further usage;
I'd probably completely rephrase this as:
The json_query converter supports the JSON types string, boolean and
number. Floating point numbers will be returned as a string. By specifying
the output_type 'int' the value will be converted to an Integer. If
conversion is not possible the json_query converter fails.
Well I would like to here also some other opinions about the wording.
I, by far, prefer Tim's proposal here, as I do not even understand the
first one, sorry Aleks, please don't feel offended :-)
Well you know my focus is to support HAProxy and therefore it's okay.
The contribution was in the past much easier, but you know time changes.
+ switch(tok) {
+ case MJSON_TOK_NUMBER:
+ if (args[1].type == ARGT_SINT) {
+ smp->data.u.sint = atoll(p);
+
+ if (smp->data.u.sint < 0 && smp->data.u.sint < JSON_INT_MIN) {
+ /* JSON integer too big negativ value */
This comment appears to be useless. It is implied by the 'if'. I also believe that
the 'sint < 0' check is not needed.
Well I prefer to document in the comment what the if is doing.
OK but then please be careful about spelling, or it will force Ilya to
send yet another spell-checker patch.
From my point of view is it necessary to check if the value is a negative
value and only then should be checked if the max '-' range is reached.
But the first one is implied by the second. It looks like a logical
error when read like this, it makes one think the author had something
different in mind. It's like writing "if (a < 0 && a < -2)". It is
particularly confusing.
Well then then this does not work anymore
http-request set-var(sess.pay_int) req.body,json_query('$.integer',"int"),add(1)
with the given defines.
#define JSON_INT_MAX ((1ULL << 53) - 1)
#define JSON_INT_MIN (0 - JSON_INT_MAX)
Because "{"integer":4}" => 5" and 5 is bigger then JSON_INT_MIN which is
(0-JSON_INT_MAX)
This sequence works because I check if the value is negative "smp->data.u.sint <
0"
and only then check if the negative max border "JSON_INT_MIN" is reached.
if (smp->data.u.sint < 0 && smp->data.u.sint < JSON_INT_MIN)
The same belongs to the positive max int.
Now when I remove the check "smp->data.u.sint < 0" every positive value is
bigger then JSON INT_MIN
and returns 0.
How about to add this information into the comments?
Maybe there is a better solution, I'm open for suggestions.
I can move the comment above the 'if'.
You have the choice as long as it's clear:
- above the if, you describe what you're testing and why
- inside the if, you describe the condition you've validated.
As it is now, it's best inside the if.
Thanks!
Willy
