Conversation
- fix spelling errors in comments, configure.ac and test output (alwas, responsibilty, specificed, entriely depens, amore than, duplicated words, Incrementes, thins, RDRANR, versbose) - JASSERT: add missing closing quote and space in failure message - fjson_object_from_fd(): report the correct function name on error - fjson_type_to_name(): print the correct upper bound of the range - printbuf.h: refer to printbuf_memappend(), printbuf_memappend_real() does not exist - json_object.h: dump uses a 1024 byte internal buffer, not 128; a child entry needs 24 bytes on x64; get_boolean/get_int/get_uint/ get_double docs were copied from json-c and did not match the implementation (no EINVAL, no NaN, no array coercion, no UINT32_MIN) - json_object_iterator.c: fix @file name
There was a problem hiding this comment.
2 issues found across 13 files
Confidence score: 3/5
json_object.c:get_intandget_int64use direct floating-point-to-integer casts despitejson_object.hdocumenting saturation and NaN-to-zero behavior, so callers may receive incorrect or platform-dependent results for out-of-range and NaN values; implement the documented checks or revise the API contract and add coverage.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="json_object.h">
<violation number="1" location="json_object.h:555">
P2: The new get_int text claims double values saturate to INT32_MAX/MIN and NaN returns 0, but json_object.c:658 does a plain `(int32_t)jso->o.c_double.value` cast with no NaN check or saturation. On x86-64 NaN casts to INT32_MIN, not 0, and out-of-range values are undefined. Since this PR removes exactly such inaccurate json-c claims, keep the statement limited to int objects.</violation>
<violation number="2" location="json_object.h:581">
P2: The new get_int64 text claims double conversions are saturated to INT64_MIN/INT64_MAX and NaN returns 0, but json_object.c:709 does a direct `(int64_t)jso->o.c_double.value` cast. The result for NaN or values beyond int64 range is undefined (typically INT64_MIN on x86-64), so the doc statement is wrong. Restore the previous plain wording or change the implementation to actually saturate.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * double objects will return their int64 conversion, saturated to | ||
| * INT64_MIN/INT64_MAX (NaN is returned as 0). Strings will be |
There was a problem hiding this comment.
P2: The new get_int64 text claims double conversions are saturated to INT64_MIN/INT64_MAX and NaN returns 0, but json_object.c:709 does a direct (int64_t)jso->o.c_double.value cast. The result for NaN or values beyond int64 range is undefined (typically INT64_MIN on x86-64), so the doc statement is wrong. Restore the previous plain wording or change the implementation to actually saturate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At json_object.h, line 581:
<comment>The new get_int64 text claims double conversions are saturated to INT64_MIN/INT64_MAX and NaN returns 0, but json_object.c:709 does a direct `(int64_t)jso->o.c_double.value` cast. The result for NaN or values beyond int64 range is undefined (typically INT64_MIN on x86-64), so the doc statement is wrong. Restore the previous plain wording or change the implementation to actually saturate.</comment>
<file context>
@@ -577,7 +578,8 @@ extern uint32_t fjson_object_get_uint(struct fjson_object *obj);
*
* The type is coerced to a int64 if the passed object is not a int64.
- * double objects will return their int64 conversion. Strings will be
+ * double objects will return their int64 conversion, saturated to
+ * INT64_MIN/INT64_MAX (NaN is returned as 0). Strings will be
* parsed as an int64. If no conversion exists then 0 is returned.
</file context>
| * double objects will return their int64 conversion, saturated to | |
| * INT64_MIN/INT64_MAX (NaN is returned as 0). Strings will be | |
| * double objects will return their int64 conversion. Strings will be | |
| * parsed as an int64. If no conversion exists then 0 is returned. |
| * INT32_MIN are returned, respectively. | ||
| * If the value is too big or too small to fit into 32-bit, INT32_MAX or | ||
| * INT32_MIN are returned, respectively. This also applies to double | ||
| * values; NaN is returned as 0. |
There was a problem hiding this comment.
P2: The new get_int text claims double values saturate to INT32_MAX/MIN and NaN returns 0, but json_object.c:658 does a plain (int32_t)jso->o.c_double.value cast with no NaN check or saturation. On x86-64 NaN casts to INT32_MIN, not 0, and out-of-range values are undefined. Since this PR removes exactly such inaccurate json-c claims, keep the statement limited to int objects.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At json_object.h, line 555:
<comment>The new get_int text claims double values saturate to INT32_MAX/MIN and NaN returns 0, but json_object.c:658 does a plain `(int32_t)jso->o.c_double.value` cast with no NaN check or saturation. On x86-64 NaN casts to INT32_MIN, not 0, and out-of-range values are undefined. Since this PR removes exactly such inaccurate json-c claims, keep the statement limited to int objects.</comment>
<file context>
@@ -546,12 +546,13 @@ extern struct fjson_object* fjson_object_new_int64(int64_t i);
- * INT32_MIN are returned, respectively.
+ * If the value is too big or too small to fit into 32-bit, INT32_MAX or
+ * INT32_MIN are returned, respectively. This also applies to double
+ * values; NaN is returned as 0.
*
* @param obj the fjson_object instance
</file context>
Summary by cubic
Corrects
fjsonAPI docs that were copied from json-c and no longer match this implementation. No API changes, but error messages and test output text change.EINVAL,NaN, array-coercion, andUINT32_MINclaims fromjson_object.hgetter docs.JASSERT,fjson_object_from_fd(), andfjson_type_to_name().printbuf.htoprintbuf_memappend()and fixes the@fileinjson_object_iterator.c.configure.ac, and expected test output.Written for commit ec0da17. Summary will update on new commits.