[ovs-dev] [PATCH] oss-fuzz: Move oss-fuzz test harnesses and fuzzer configs to ovs source repo

Bhargava Shastry bshastry at sect.tu-berlin.de
Fri Jul 6 09:06:50 UTC 2018


Hi Ben,

> We do need a Signed-off-by on any patch.  Can you provide one?

Sure, this should not be a problem. I'm not sure how the patch was
generated from my pull request. Since you prefer patches, I can switch
to git-format-patch or something like that to take this forward. I will
do this once I have a new patch ready that addresses your comments.

> It would be helpful to include a little bit of explanation of the
> purpose of the patch in the body of the commit message.  I guess you did
> that in the top-level pull request at
> https://github.com/openvswitch/ovs/pull/242, but it doesn't show up in
> the patch itself.

Ack.

> Normally I expect new code to be mentioned in some makefile.  It's not
> obvious to me how this gets built.  Does it get built by oss-fuzz itself
> somehow?  Or are you building it by hand?  Or something else?

The new code (test harnesses) get built from a build script inside the
oss-fuzz repo:
https://github.com/google/oss-fuzz/blob/master/projects/openvswitch/build.sh

Please bear in mind that the build script is invoked inside a Docker
container defined here:
https://github.com/google/oss-fuzz/blob/master/projects/openvswitch/Dockerfile

Also, please note that this patch is aimed at moving the test harnesses
you are going to find in oss-fuzz to OvS so that
oss-fuzz/project/openvswitch contains the bare minimum: project metadata
(project.yaml), Dockerfile, and build script.

> This causes some build breakage at "make" time.  I'll walk you through
> it.  First, we normally expect #include <config.h> at the beginning of
> each file.  Since I don't understand how this gets built, maybe that's
> not appropriate (but in that case we need to update our blacklist for
> this rule):
> 
>     tests/oss-fuzz/flow_extract_target.c
>     tests/oss-fuzz/json_parser_target.c
>     tests/oss-fuzz/ofp_print_target.c
>     See above for list of violations of the rule that
>     every C source file must #include <config.h>.

Is this comment still valid in the light of my comment on build process?

> Please change tabs to spaces:
> 
>     tests/oss-fuzz/ofp_print_target.c
>     See above for files that use tabs for indentation.
>     Please use spaces instead.

Ack.

> The new files need to get mentioned in an automake.mk, at least in
> EXTRA_DIST, to ensure that "make dist" will put them into the tarball:
> 
>     The following files are in git but not the distribution:
>     tests/oss-fuzz/config/flow_extract_fuzzer.options
>     tests/oss-fuzz/config/json_parser_fuzzer.options
>     tests/oss-fuzz/config/ofp_print_fuzzer.options
>     tests/oss-fuzz/config/ovs.dict
>     tests/oss-fuzz/flow_extract_target.c
>     tests/oss-fuzz/json_parser_target.c
>     tests/oss-fuzz/ofp_print_target.c

There are several automake files to choose from. How do I do this?

> checkpatch has the following to say about coding style; it would be nice
> to fix all of it:
> 
>     ERROR: Inappropriate spacing in pointer declaration
>     WARNING: Line lacks whitespace around operator
>     #357 FILE: tests/oss-fuzz/flow_extract_:s.c:4:
>     int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
> 
>     ERROR: Inappropriate spacing in pointer declaration
>     WARNING: Line lacks whitespace around operator
>     #388 FILE: tests/oss-fuzz/json_parser_target.c:17:
>     int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
> 
>     ERROR: Inappropriate bracing around statement
>     #390 FILE: tests/oss-fuzz/json_parser_target.c:19:
>         if ((size == 0) || (data[size-1] != '\0')) return 0;
> 
>     WARNING: Line lacks whitespace around operator
>     #392 FILE: tests/oss-fuzz/json_parser_target.c:21:
>         struct json *j1,*j2;
> 
>     WARNING: Line has trailing whitespace
>     #402 FILE: tests/oss-fuzz/json_parser_target.c:31:
> 
>     WARNING: Line has trailing whitespace
>     #414 FILE: tests/oss-fuzz/json_parser_target.c:43:
> 
>     ERROR: Inappropriate spacing in pointer declaration
>     #446 FILE: tests/oss-fuzz/ofp_print_target.c:6:
>     int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
> 
>     ERROR: Inappropriate bracing around statement
>     #450 FILE: tests/oss-fuzz/ofp_print_target.c:10:
>         if (size < sizeof(struct ofp_header)) return 0;

Ack.

Regards,
Bhargava


More information about the dev mailing list