Zygo / bees

Best-Effort Extent-Same, a btrfs dedupe agent
GNU General Public License v3.0
647 stars 55 forks source link

Issues reported by static analysis #168

Closed telans closed 3 years ago

telans commented 3 years ago

Static analysis through scan-build and Clang/LLVM 12.0.0-rc1 (alpha checkers):

error.cc:54:3: warning: This statement is never executed [alpha.deadcode.UnreachableCode]
                try {
                ^~~~~
error.cc:66:3: warning: This statement is never executed [alpha.deadcode.UnreachableCode]
                try {
                ^~~~~
city.cc:197:3: warning: Duplicate code detected [alpha.clone.CloneChecker]
  uint32 a0 = Rotate32(Fetch32(s + len - 4) * c1, 17) * c2;
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
city.cc:198:3: note: Similar code here
  uint32 a1 = Rotate32(Fetch32(s + len - 8) * c1, 17) * c2;
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
city.cc:388:5: warning: Duplicate code detected [alpha.clone.CloneChecker]
    x = Rotate(x + y + v.first + Fetch64(s + 8), 37) * k1;
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
city.cc:471:5: note: Similar code here
    x = Rotate(x + y + v.first + Fetch64(s + 8), 37) * k1;
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from extentwalker.cc:5:
../include/crucible/fs.h:214:10: warning: Casting a non-structure type to a structure type and accessing a field can lead to memory access errors or data corruption [alpha.core.CastToStruct]
                return reinterpret_cast<const T*>(v.data() + offset);
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...

bees-scan-build.tar.gz

See the attached file/html pages for an interactive view of function paths taken (or just run scan-build -V make with alpha checkers yourself). Not sure if any of these are actually issues but I thought you may be interested.

Zygo commented 3 years ago

None of these seem particularly relevant. Most of them are in code that buries the questionable stuff in a private class method, specifically because it is questionable but a necessary part of the kernel interface. I'm not touching code for imported hash functions.

The "duplicated code" warnings are more entertainment than utility.