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

Ben Pfaff blp at ovn.org
Fri Jul 6 17:28:19 UTC 2018


On Fri, Jul 06, 2018 at 11:06:50AM +0200, Bhargava Shastry wrote:
> > 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.

I did a "git pull" on your repo, then "git send-email" generated the
patch.

> 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.

Thanks!

> > 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

Is that the preferred way to do it?  It seems a little ad hoc.  Another
way would be to add a target to the OVS build tree, so that the script
could just become something like

        ./boot.sh && ./configure && make -j$(nproc) tests/oss-fuzz/fuzzer

or whatever.

But, maybe this ad hoc build method is conventional for oss-fuzz?  I'm
not familiar with their usual processes.

> > 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?

Well, either they have to have #include <config.h> or we need to
blacklist these files, one or the other.  The former is probably
harmless and possibly helpful.

> > 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?

I'd add a new tests/oss-fuzz/automake.mk and then include that in
tests/automake.mk.

Thanks!

Ben


More information about the dev mailing list