Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.6/f/json parser unit test #56

Merged
merged 11 commits into from
Jan 7, 2014
Merged

3.6/f/json parser unit test #56

merged 11 commits into from
Jan 7, 2014

Conversation

bazsi
Copy link
Collaborator

@bazsi bazsi commented Jan 2, 2014

No description provided.

@algernon
Copy link
Contributor

algernon commented Jan 2, 2014

There's still a segmentation fault in the test case. I see it on Travis, but have not tried it locally yet.

@algernon
Copy link
Contributor

algernon commented Jan 2, 2014

I think test_json_parser_validate_type_representation() is failing, as that's the only test that contains a NULL value, which is mentioned in the error message:

[2014-01-02T14:11:06.194677] Unparsable JSON stream encountered; error='null expected'
/bin/bash: line 5:  3474 Segmentation fault      ${dir}$tst
FAIL: modules/json/tests/test_jsonparser

I suspect this may have to do something with Travis having json-c 0.9, while I have 0.11 locally. I'll try to reproduce it on a Precise chroot.

modules/json builds libjson-plugin.la instead of simply libjson.la because
of library name clashes (see b9125dd), however the per-subdirectory
Makefile contained a heuristic to find out what to build at install time.

This heuristic failed for modules/json/ as it didn't meet the convention,
and this patch fixes that.

Signed-off-by: Balazs Scheidler <bazsi@balabit.hu>
As that unit test only covers the $(format-json) template function.

Signed-off-by: Balazs Scheidler <bazsi@balabit.hu>
As the unit tests don't cause the recompile the json plugin.

Signed-off-by: Balazs Scheidler <bazsi@balabit.hu>
As we don't use that type for anything but the return type and causes
additional casts on the call sites.

Signed-off-by: Balazs Scheidler <bazsi@balabit.hu>
When a json-parser() is referenced from multiple points in the configuration,
it is cloned. However the clone() operation could reference a NULL pointer
if marker was not set.

This patch fixes this issue.

Signed-off-by: Balazs Scheidler <bazsi@balabit.hu>
Old versions of libjson-c used to store C NULL value in the returned
struct json_object, which got dereferenced by json-parser.

Although newer json-c versions seems to have fixed this issue
(0.11 tested here) add a workaround for this case.

Signed-off-by: Balazs Scheidler <bazsi@balabit.hu>
This patch implements a unit test for the json-parser() parser object.

Signed-off-by: Balazs Scheidler <bazsi@balabit.hu>
Signed-off-by: Balazs Scheidler <bazsi@balabit.hu>
Signed-off-by: Balazs Scheidler <bazsi@balabit.hu>
For consistency with the rest of the syslog-ng codebase.

Signed-off-by: Balazs Scheidler <bazsi@balabit.hu>
@bazsi
Copy link
Collaborator Author

bazsi commented Jan 6, 2014

I have updated a branch now with the following changes:

  1. the segfault should be gone, I compiled an old json-c locally and the root cause was that json-c 0.9 actually stores a NULL value instead of a null json object. Handling the NULL makes the segfault go away for me.

  2. it contains a couple of follow-up changes that the unit test permitted me to make. Instead of adding those to the nodejs-support branch, which I'm submitting for merging in just a minute.

@algernon
Copy link
Contributor

algernon commented Jan 6, 2014

Nice catch with json-c! I skimmed through the patches, and they all look good. Fingers crossed for Travis thinking similarly.

bazsi added a commit that referenced this pull request Jan 7, 2014
@bazsi bazsi merged commit 01067bb into master Jan 7, 2014
@bazsi bazsi deleted the 3.6/f/json-parser-unit-test branch January 7, 2014 18:08
szemere pushed a commit to szemere/syslog-ng that referenced this pull request Jul 2, 2020
tools: add Criterion && Astyle description
bazsi pushed a commit to bazsi/syslog-ng that referenced this pull request May 17, 2024
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.

None yet

2 participants