On 15 Dec 2015, at 7:37, Donald Sharp wrote:
Martin -
As discussed in the meeting today here is an example of a bogus issue:
https://ci1.netdef.org/artifact/QUAGGA-QMASTER/shared/build-82/static_analysis/report-8a9e85.html#EndPath
Basically the error message boils down to this code construct:
struct prefix p;
if (afi == v4)
set p appropriately
else if (afi == v6)
set p appropriately
if (p.prefixlen) <------- If afi is not v4 or v6 then prefix length
is
not going to be set.
I think in this case it is correct for Clang to warn and I would expect
any
other static analyzer to flag this as well.
If you would thought that afi can really only have a value of v4 or v6,
then you could have written it as
if (afi == v4)
set p appropriately
else /* afi == v6 */
set p appropriately
However, by writing it as above, whoever wrote it might have thought
that
(at least in future or failure cases) it could have another value and
wanted
to protect from it.
If this is a very unlikely case, then why not write it like this ?
if (afi == v4)
set p appropriately
else if (afi == v6)
set p appropriately
else
assert (and blow up)
Or if this is a bit more likely to happen then return a failure code
from the function
(like Paul suggested)
I spent a few minutes looking at clang and saw no good way to mark the
code
in plist.c as ok.
Workarounds for common clang errors are described here:
http://clang-analyzer.llvm.org/faq.html
I think their recommendation is to add a custom assert for this case
Regards,
Martin Winter
On Mon, Dec 7, 2015 at 7:26 PM, Martin Winter
<[email protected]
wrote:
Just wanted to give a quick update on what I’m up the past few
days:
1) Added OpenBSD 5.8 to my CI Testbed (currently works fine)
============================================================
See
https://git-us.netdef.org/projects/OSR/repos/ci-files/browse/doc/Quagga_OpenBSD58.md
for details on how I build it. Packages are built as well (but not
really
tested)
OpenBSD adds a few patches when they build Quagga - probably need to
review them if
they make sense to integrate into the mainline. I’m currently
ignoring the
patches
in my test packages I’m building
(Thanks to Peter Hessler from the OpenBSD team for a few starter
hints)
2) Added Clang Static analyzer
============================================================
I’m not yet sure what to do with the output (i.e. how/when I should
flag a
build
as “failed” based on the output.
For all the major plans on out CI system, this is now added and the
report
archived.
You’ll find them under the Artifacts. Click on a recent run number
on the
CI plan
(i.e. on https://ci1.netdef.org/browse/QUAGGA-QMASTER), then on
Artifacts
and you
should see a link for the Static Analyzer Results)
Current Master:
https://ci1.netdef.org/browse/QUAGGA-QMASTER-82/artifact/shared/static_analysis
Current Proposed-5 branch:
https://ci1.netdef.org/browse/QUAGGA-QMASTER7-2/artifact/shared/static_analysis
Suggestion on what to track / how to track / how to proceed are
welcome.
3) Automated Patchwork Testing (and reporting)
============================================================
Going to resume sending the automated patches (starting from patches
submitted a week ago),
but still not sure on how to deal with patches which do not cleanly
apply
against the
master branch. If someone has a suggestion, then please feel free to
speak
up.
I’m wondering how a “human” guesses the correct branch for
testing (i.e.
anyone here
on the mailing list picking up patches and testing them by hand)
In general, I always run it automatically, just didn’t send the
email in
the past weeks.
If you want to see a specific results, then just access them
directly:
i.e. for any run with Patchwork 1599 in it:
https://ci1.netdef.org/browse/label/patchwork_id=1599
Ideas:
a) Community generally rejects patches against older commits or other
branches (and I
just report them as failures
b) Somehow the commit message includes the branch to test against.
c) Try master first and if it fails to apply the patch, automatically
try
other branches
d) Get submitters to tag the patches someway if they are NOT against
current master
Opinions?
_______________________________________________
Quagga-dev mailing list
[email protected]
https://lists.quagga.net/mailman/listinfo/quagga-dev
_______________________________________________
Quagga-dev mailing list
[email protected]
https://lists.quagga.net/mailman/listinfo/quagga-dev
