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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
