| Message ID | 1467043900-121140-1-git-send-email-bhanuprakash.bodireddy@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
This is pretty cool. I tested it and have some comments. On Mon, Jun 27, 2016 at 9:11 AM, Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com> wrote: > Clang Static Analyzer is a source code analysis tool to find bugs. This > patch adds make target to trigger static analysis using below commands. > > ./boot.sh > ./configure --with-dpdk(for configuring DPDK datapath) > make clang-analyze > scan-view --host=<ip address> --port <PORT> > $OVS_DIR>/clang-analyzer-results/yyyy-mm-dd-114251-1027-1> --allow-all-hosts > > Results can be viewed on browser: http://<ip address>:<PORT>/ > > Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com> > --- > Makefile.am | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/Makefile.am b/Makefile.am > index 8cb8523..ac96be6 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -400,6 +400,16 @@ ovsext_clean: datapath-windows/ovsext.sln > endif > .PHONY: ovsext > > +clang-analyze: clean > + @if which clang scan-build > /dev/null 2>&1; then \ > + $(MKDIR_P) "$(srcdir)/clang-analyzer-results" || exit 1; \ > + scan-build -o $(srcdir)/clang-analyzer-results --use-analyzer=/usr/bin/clang \ Since we have valgrind/helgrind results under tests dir, maybe output to ''$(srcdir)/tests/clang-analyzer-results". > + make -j || exit 1; \ "make -j" creates lots of jobs and hangs my system. Maybe just use 'make' and let people to optimize if they want. Regards, William
----- Original Message ----- > From: "Bhanuprakash Bodireddy" <bhanuprakash.bodireddy@intel.com> > To: dev@openvswitch.org > Sent: Monday, June 27, 2016 12:11:40 PM > Subject: [ovs-dev] [PATCH] Makefile.am: Add clang static analysis support > > Clang Static Analyzer is a source code analysis tool to find bugs. This > patch adds make target to trigger static analysis using below commands. > > ./boot.sh > ./configure --with-dpdk(for configuring DPDK datapath) > make clang-analyze > scan-view --host=<ip address> --port <PORT> > $OVS_DIR>/clang-analyzer-results/yyyy-mm-dd-114251-1027-1> > --allow-all-hosts > > Results can be viewed on browser: http://<ip address>:<PORT>/ > > Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com> > --- > Makefile.am | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/Makefile.am b/Makefile.am > index 8cb8523..ac96be6 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -400,6 +400,16 @@ ovsext_clean: datapath-windows/ovsext.sln > endif > .PHONY: ovsext > > +clang-analyze: clean > + @if which clang scan-build > /dev/null 2>&1; then \ > + $(MKDIR_P) "$(srcdir)/clang-analyzer-results" || exit 1; \ > + scan-build -o $(srcdir)/clang-analyzer-results > --use-analyzer=/usr/bin/clang \ > + make -j || exit 1; \ > + else \ > + echo -e "Unable to find clang/scan-build, Install clang,clang-analyzer > packages"; \ > + fi > +.PHONY: clang-analyze > + > dist-hook: $(DIST_HOOKS) > all-local: $(ALL_LOCAL) > clean-local: $(CLEAN_LOCAL) > -- > 2.4.11 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev > LGTM, I tried it out with no issues found. a couple of small suggestions: - It would be good to add some text to the "Build Requirements" section of INSTALL.md, mentioning clang-analyzer where clang is already listed. - It might also be nice to have a sentence or two somewhere in INSTALL.md about how to use this feature.
>-----Original Message----- >From: William Tu [mailto:u9012063@gmail.com] >Sent: Monday, June 27, 2016 6:57 PM >To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy@intel.com> >Cc: <dev@openvswitch.org> <dev@openvswitch.org> >Subject: Re: [ovs-dev] [PATCH] Makefile.am: Add clang static analysis support > >This is pretty cool. I tested it and have some comments. > >On Mon, Jun 27, 2016 at 9:11 AM, Bhanuprakash Bodireddy ><bhanuprakash.bodireddy@intel.com> wrote: >> Clang Static Analyzer is a source code analysis tool to find bugs. >> This patch adds make target to trigger static analysis using below commands. >> >> ./boot.sh >> ./configure --with-dpdk(for configuring DPDK datapath) make >> clang-analyze scan-view --host=<ip address> --port <PORT> >> $OVS_DIR>/clang-analyzer-results/yyyy-mm-dd-114251-1027-1> >> --allow-all-hosts >> >> Results can be viewed on browser: http://<ip address>:<PORT>/ >> >> Signed-off-by: Bhanuprakash Bodireddy >> <bhanuprakash.bodireddy@intel.com> >> --- >> Makefile.am | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/Makefile.am b/Makefile.am index 8cb8523..ac96be6 100644 >> --- a/Makefile.am >> +++ b/Makefile.am >> @@ -400,6 +400,16 @@ ovsext_clean: datapath-windows/ovsext.sln endif >> .PHONY: ovsext >> >> +clang-analyze: clean >> + @if which clang scan-build > /dev/null 2>&1; then \ >> + $(MKDIR_P) "$(srcdir)/clang-analyzer-results" || exit 1; \ >> + scan-build -o $(srcdir)/clang-analyzer-results >> +--use-analyzer=/usr/bin/clang \ > >Since we have valgrind/helgrind results under tests dir, maybe output to >''$(srcdir)/tests/clang-analyzer-results". I agree to this. I will do this in v2 patch. > >> + make -j || exit 1; \ > >"make -j" creates lots of jobs and hangs my system. Maybe just use 'make' >and let people to optimize if they want. Thanks for testing and pointing this out. I had intentionally used 'make -j' to speed up the analysis and tested it on my target with 28 cores. I shall update the patch to remove '-j' flag to prevent potential hangs. > >Regards, >William
>-----Original Message----- >From: Lance Richardson [mailto:lrichard@redhat.com] >Sent: Monday, June 27, 2016 7:34 PM >To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy@intel.com> >Cc: dev@openvswitch.org >Subject: Re: [ovs-dev] [PATCH] Makefile.am: Add clang static analysis support > > > >----- Original Message ----- >> From: "Bhanuprakash Bodireddy" <bhanuprakash.bodireddy@intel.com> >> To: dev@openvswitch.org >> Sent: Monday, June 27, 2016 12:11:40 PM >> Subject: [ovs-dev] [PATCH] Makefile.am: Add clang static analysis >> support >> >> Clang Static Analyzer is a source code analysis tool to find bugs. >> This patch adds make target to trigger static analysis using below commands. >> >> ./boot.sh >> ./configure --with-dpdk(for configuring DPDK datapath) make >> clang-analyze scan-view --host=<ip address> --port <PORT> >> $OVS_DIR>/clang-analyzer-results/yyyy-mm-dd-114251-1027-1> >> --allow-all-hosts >> >> Results can be viewed on browser: http://<ip address>:<PORT>/ >> >> Signed-off-by: Bhanuprakash Bodireddy >> <bhanuprakash.bodireddy@intel.com> >> --- >> Makefile.am | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/Makefile.am b/Makefile.am index 8cb8523..ac96be6 100644 >> --- a/Makefile.am >> +++ b/Makefile.am >> @@ -400,6 +400,16 @@ ovsext_clean: datapath-windows/ovsext.sln endif >> .PHONY: ovsext >> >> +clang-analyze: clean >> + @if which clang scan-build > /dev/null 2>&1; then \ >> + $(MKDIR_P) "$(srcdir)/clang-analyzer-results" || exit 1; \ >> + scan-build -o $(srcdir)/clang-analyzer-results >> --use-analyzer=/usr/bin/clang \ >> + make -j || exit 1; \ >> + else \ >> + echo -e "Unable to find clang/scan-build, Install >> +clang,clang-analyzer >> packages"; \ >> + fi >> +.PHONY: clang-analyze >> + >> dist-hook: $(DIST_HOOKS) >> all-local: $(ALL_LOCAL) >> clean-local: $(CLEAN_LOCAL) >> -- >> 2.4.11 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> http://openvswitch.org/mailman/listinfo/dev >> > >LGTM, I tried it out with no issues found. Thanks for testing the patch. > a couple of small suggestions: > - It would be good to add some text to the "Build Requirements" section of > INSTALL.md, mentioning clang-analyzer where clang is already listed. > - It might also be nice to have a sentence or two somewhere in INSTALL.md >about > how to use this feature. I agree to your suggestion. I worked on refactoring the install guide in to Beginner and Advanced guides and submitted v7 recently. I have a section 9 in the ADVANCED install guide that talks about static analysis. Please check the rendered form here. https://github.com/bbodired/ovs/blob/master/INSTALL.DPDK-ADVANCED.md v7 patch: http://openvswitch.org/pipermail/dev/2016-June/thread.html Regards, Bhanu Prakash.
----- Original Message ----- > From: "Bhanuprakash Bodireddy" <bhanuprakash.bodireddy@intel.com> > To: "Lance Richardson" <lrichard@redhat.com> > Cc: dev@openvswitch.org > Sent: Monday, June 27, 2016 3:23:26 PM > Subject: RE: [ovs-dev] [PATCH] Makefile.am: Add clang static analysis support > > >-----Original Message----- > >From: Lance Richardson [mailto:lrichard@redhat.com] > >Sent: Monday, June 27, 2016 7:34 PM > >To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy@intel.com> > >Cc: dev@openvswitch.org > >Subject: Re: [ovs-dev] [PATCH] Makefile.am: Add clang static analysis > >support > > > > > > > >----- Original Message ----- > >> From: "Bhanuprakash Bodireddy" <bhanuprakash.bodireddy@intel.com> > >> To: dev@openvswitch.org > >> Sent: Monday, June 27, 2016 12:11:40 PM > >> Subject: [ovs-dev] [PATCH] Makefile.am: Add clang static analysis > >> support > >> > >> Clang Static Analyzer is a source code analysis tool to find bugs. > >> This patch adds make target to trigger static analysis using below > >> commands. > >> > >> ./boot.sh > >> ./configure --with-dpdk(for configuring DPDK datapath) make > >> clang-analyze scan-view --host=<ip address> --port <PORT> > >> $OVS_DIR>/clang-analyzer-results/yyyy-mm-dd-114251-1027-1> > >> --allow-all-hosts > >> > >> Results can be viewed on browser: http://<ip address>:<PORT>/ > >> > >> Signed-off-by: Bhanuprakash Bodireddy > >> <bhanuprakash.bodireddy@intel.com> > >> --- > >> Makefile.am | 10 ++++++++++ > >> 1 file changed, 10 insertions(+) > >> > >> diff --git a/Makefile.am b/Makefile.am index 8cb8523..ac96be6 100644 > >> --- a/Makefile.am > >> +++ b/Makefile.am > >> @@ -400,6 +400,16 @@ ovsext_clean: datapath-windows/ovsext.sln endif > >> .PHONY: ovsext > >> > >> +clang-analyze: clean > >> + @if which clang scan-build > /dev/null 2>&1; then \ > >> + $(MKDIR_P) "$(srcdir)/clang-analyzer-results" || exit 1; \ > >> + scan-build -o $(srcdir)/clang-analyzer-results > >> --use-analyzer=/usr/bin/clang \ > >> + make -j || exit 1; \ > >> + else \ > >> + echo -e "Unable to find clang/scan-build, Install > >> +clang,clang-analyzer > >> packages"; \ > >> + fi > >> +.PHONY: clang-analyze > >> + > >> dist-hook: $(DIST_HOOKS) > >> all-local: $(ALL_LOCAL) > >> clean-local: $(CLEAN_LOCAL) > >> -- > >> 2.4.11 > >> > >> _______________________________________________ > >> dev mailing list > >> dev@openvswitch.org > >> http://openvswitch.org/mailman/listinfo/dev > >> > > > >LGTM, I tried it out with no issues found. > > Thanks for testing the patch. > > > a couple of small suggestions: > > - It would be good to add some text to the "Build Requirements" section > > of > > INSTALL.md, mentioning clang-analyzer where clang is already listed. > > - It might also be nice to have a sentence or two somewhere in INSTALL.md > >about > > how to use this feature. > > I agree to your suggestion. I worked on refactoring the install guide in to > Beginner and Advanced guides and submitted v7 recently. > I have a section 9 in the ADVANCED install guide that talks about static > analysis. Please check the rendered form here. > https://github.com/bbodired/ovs/blob/master/INSTALL.DPDK-ADVANCED.md > That looks good, but I wonder if the the advanced DPDK installation guide is really the right place for it... clang-analyzer seems useful for both DPDK and non-DPDK builds. Thanks, Lance > v7 patch: http://openvswitch.org/pipermail/dev/2016-June/thread.html > > Regards, > Bhanu Prakash. >
diff --git a/Makefile.am b/Makefile.am index 8cb8523..ac96be6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -400,6 +400,16 @@ ovsext_clean: datapath-windows/ovsext.sln endif .PHONY: ovsext +clang-analyze: clean + @if which clang scan-build > /dev/null 2>&1; then \ + $(MKDIR_P) "$(srcdir)/clang-analyzer-results" || exit 1; \ + scan-build -o $(srcdir)/clang-analyzer-results --use-analyzer=/usr/bin/clang \ + make -j || exit 1; \ + else \ + echo -e "Unable to find clang/scan-build, Install clang,clang-analyzer packages"; \ + fi +.PHONY: clang-analyze + dist-hook: $(DIST_HOOKS) all-local: $(ALL_LOCAL) clean-local: $(CLEAN_LOCAL)
Clang Static Analyzer is a source code analysis tool to find bugs. This patch adds make target to trigger static analysis using below commands. ./boot.sh ./configure --with-dpdk(for configuring DPDK datapath) make clang-analyze scan-view --host=<ip address> --port <PORT> $OVS_DIR>/clang-analyzer-results/yyyy-mm-dd-114251-1027-1> --allow-all-hosts Results can be viewed on browser: http://<ip address>:<PORT>/ Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com> --- Makefile.am | 10 ++++++++++ 1 file changed, 10 insertions(+)