weaselab/weaseljson
2
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Handle null parser in WeaselJsonParser_parse #54

Merged
andrew merged 3 commits from null-parser into main 2026-07-15 22:04:01 +00:00
Conversation 2 Commits 3 Files Changed 5
+3 -4
3 changed files with 3 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 6520039dc2 - Show all commits
include/weaseljson.h
+1 -1
View File
2
 @@ -67,7 +67,7 @@ void WeaselJsonParser_destroy(WeaselJsonParser *parser);
/** Incrementally parse `len` more bytes starting at `buf`. `buf` may be /** Incrementally parse `len` more bytes starting at `buf`. `buf` may be
* modified. Call with `len` 0 to indicate end of data. `buf` may be null if * modified. Call with `len` 0 to indicate end of data. `buf` may be null if
* `len` is 0. `len` must not be negative; a negative length is treated as a * `len` is 0. `len` must not be negative; a negative length is treated as a
* rejected input. */ * rejected input. Returns WeaselJson_NULL if parser is null */
andrew marked this conversation as resolved Outdated
weaselbot commented 2026-07-15 18:01:40 +00:00
Outdated
Review
Copy Link
Copy Source

The doc comment covers the negative-length behavior but not the null-parser behavior added in this PR. Please add a sentence, e.g. "If parser is null, parse returns WeaselJson_NULL (or WeaselJson_REJECT) without dereferencing it.", so the header stays self-consistent with the implementation.

The doc comment covers the negative-length behavior but not the null-parser behavior added in this PR. Please add a sentence, e.g. "If parser is null, parse returns WeaselJson_NULL (or WeaselJson_REJECT) without dereferencing it.", so the header stays self-consistent with the implementation.
WeaselJsonStatus WeaselJsonParser_parse(WeaselJsonParser *parser, char *buf, WeaselJsonStatus WeaselJsonParser_parse(WeaselJsonParser *parser, char *buf,
int len); int len);
src/test.cpp
+1 -2
View File
@@ -331,8 +331,7 @@ TEST_CASE("parse rejects negative length") {
} }
TEST_CASE("Calling parse with nullptr doesn't crash") { TEST_CASE("Calling parse with nullptr doesn't crash") {
char buf[10] = "hello"; REQUIRE(WeaselJsonParser_parse(nullptr, nullptr, 0) == WeaselJson_NULL);
REQUIRE(WeaselJsonParser_parse(nullptr, buf, -1) == WeaselJson_NULL);
} }
andrew marked this conversation as resolved
weaselbot commented 2026-07-15 18:01:40 +00:00
Review
Copy Link
Copy Source

This couples the null-parser check with the negative-length case. Because the null check runs first it returns WeaselJson_NULL, but it would be clearer to use a valid length (e.g. WeaselJsonParser_parse(nullptr, buf, 5)) so the test isolates the null-parser behavior. It'd also be nice to mirror issue #51's exact reproduction - WeaselJsonParser_parse(nullptr, nullptr, 0) == WeaselJson_NULL - to document that buf=nullptr,len=0 is handled too.

This couples the null-parser check with the negative-length case. Because the null check runs first it returns WeaselJson_NULL, but it would be clearer to use a valid length (e.g. WeaselJsonParser_parse(nullptr, buf, 5)) so the test isolates the null-parser behavior. It'd also be nice to mirror issue #51's exact reproduction - WeaselJsonParser_parse(nullptr, nullptr, 0) == WeaselJson_NULL - to document that buf=nullptr,len=0 is handled too.
TEST_CASE("streaming") { testStreaming(json); } TEST_CASE("streaming") { testStreaming(json); }
src/validate.cpp
+1 -1
View File
@@ -34,7 +34,7 @@ int main(int argc, char **argv) {
case WeaselJson_OVERFLOW: case WeaselJson_OVERFLOW:
return 1; return 1;
case WeaselJson_NULL: case WeaselJson_NULL:
fprintf(stderr, "Could not create parser\n"); fprintf(stderr, "parse called with a null parser\n");
andrew marked this conversation as resolved Outdated
weaselbot commented 2026-07-15 18:01:40 +00:00
Outdated
Review
Copy Link
Copy Source

This message ("Could not create parser") describes a different condition than the status: it's reached when parse() is called with a null pointer, not when create() failed. Something like fprintf(stderr, "parse called with a null parser\n"); would be clearer. (In this program the two coincide because parser.get() is only null when create failed, but the wording is still misleading relative to the status name.)

This message ("Could not create parser") describes a different condition than the status: it's reached when parse() is called with a null pointer, not when create() failed. Something like fprintf(stderr, "parse called with a null parser\n"); would be clearer. (In this program the two coincide because parser.get() is only null when create failed, but the wording is still misleading relative to the status name.)
return 1; return 1;
} }
if (l == 0) { if (l == 0) {