Skip to content

docs: fix typos and outdated API documentation - #181

Open
prownd wants to merge 1 commit into
rsyslog:mainfrom
prownd:docs-fix-typos-and-outdated-API-documentation-s01
Open

prownd wants to merge 1 commit into
rsyslog:mainfrom
prownd:docs-fix-typos-and-outdated-API-documentation-s01

Conversation

@prownd

@prownd prownd commented Sep 15, 2026

Copy link
Copy Markdown
  • 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

Summary by cubic

Corrects fjson API docs that were copied from json-c and no longer match this implementation. No API changes, but error messages and test output text change.

  • Removes incorrect EINVAL, NaN, array-coercion, and UINT32_MIN claims from json_object.h getter docs.
  • Updates the documented dump buffer size to 1024 bytes and child entry size to 24 bytes on x64.
  • Fixes error messages in JASSERT, fjson_object_from_fd(), and fjson_type_to_name().
  • Points printbuf.h to printbuf_memappend() and fixes the @file in json_object_iterator.c.
  • Fixes spelling in comments, configure.ac, and expected test output.

Written for commit ec0da17. Summary will update on new commits.

Review in cubic

- 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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 13 files

Confidence score: 3/5

  • json_object.c:get_int and get_int64 use direct floating-point-to-integer casts despite json_object.h documenting 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

Comment thread json_object.h
Comment on lines +581 to +582
* double objects will return their int64 conversion, saturated to
* INT64_MIN/INT64_MAX (NaN is returned as 0). Strings will be

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
* 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.

Comment thread json_object.h
* 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant