-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
json-c: run fuzz test from upstream repository #12741
base: master
Are you sure you want to change the base?
Conversation
simonresch is a new contributor to projects/json-c. The PR must be approved by known contributors before it can be merged. The past contributors are: devtty1er, derwolfe, Dor1s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you assist with getting maintainer approval?
projects/json-c/build.sh
Outdated
@@ -20,11 +20,11 @@ cmake -DBUILD_SHARED_LIBS=OFF .. | |||
make -j$(nproc) | |||
cd .. | |||
|
|||
cp $SRC/*.dict $OUT/ | |||
cp fuzz/*.dict $OUT/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the "fuzz/" directory, and why do we want to use it instead of $SRC? I don't see it mentioned at eg https://github.com/google/oss-fuzz/blob/master/docs/getting-started/new_project_guide.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is the fuzz directory inside the json-c repository. The build script is executed inside a checkout of json-c (see https://github.com/google/oss-fuzz/blob/master/projects/json-c/Dockerfile#L20). I can change this to $SRC/json-c/fuzz
for a more explicit path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hawicz Anything else I could add to clear up this PR?
If you want to try the changes out for yourself you can clone this branch via git clone -b use-upstream-json-c-fuzz-test [email protected]:simonresch/oss-fuzz.git
. Then inside the oss-fuzz
directory run python3 infra/helper.py introspector json-c
to build all fuzz tests and briefly execute them. With the changes from this PR it should execute all fuzz tests from https://github.com/json-c/json-c/tree/master/fuzz instead of the single fuzz test that is currently configured in the oss-fuzz repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
d7fa10a
to
6951303
Compare
This commit removes the json-c fuzz test from the OSS-Fuzz repo in favor of running the fuzz test that is maintained directly in the json-c repository.
6951303
to
b6e26ec
Compare
@DavidKorczynski could you take a look at this PR? Maintainer approval is given here. |
This commit removes the json-c fuzz test from the OSS-Fuzz repo in favor of running the fuzz test that is maintained directly in the json-c repository.