Page MenuHomeFreeBSD

fusefs: coverity cleanup in the tests
ClosedPublic

Authored by asomers on Aug 28 2019, 5:24 PM.

Details

Summary

fusefs: coverity cleanup in the tests

Address the following defects reported by Coverity:

  • Structurally dead code (CID 1404366): set m_quit before FAIL, not after
  • Unchecked return value of sysctlbyname (CID 1404321)
  • Unchecked return value of stat(2) (CID 1404471)
  • Unchecked return value of open(2) (CID 1404402, 1404529)
  • Unchecked return value of dup(2) (CID 1404478)
  • Buffer overflows. These are all false positives caused by the fact that Coverity thinks I'm using a buffer to store strings, when in fact I'm really just using it to store a byte array that happens to be initialized with a string. I'm changing the type from char to uint8_t in the hopes that it will placate Coverity. (CID 1404338, 1404350, 1404367, 1404376, 1404379, 1404381, 1404388, 1404403, 1404425, 1404433, 1404434, 1404474, 1404480, 1404484, 1404503, 1404505)
  • False positive file descriptor leak. I'm going to try to fix this with Coverity modeling, but I'll also change an EXPECT to ASSERT so we don't perform meaningless assertions after the failure. (CID 1404320, 1404324, 1404440, 1404445).
  • Unannotated file descriptor leak. This will be followed up by a Coverity modeling change. (CID 1404326, 1404334, 1404336, 1404357, 1404361, 1404372, 1404391, 1404395, 1404409, 1404430, 1404448, 1404451, 1404455, 1404457, 1404458, 1404460)
  • Uninitialized variables in C++ constructors (CID 1404327, 1404346). In the case of m_maxphys, this actually led to part of the FUSE_INIT's response being set to stack garbage during the WriteCluster::clustering test.
  • Uninitialized sun_len field in struct sockaddr_un (CID 1404330, 1404371, 1404429).

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

asomers created this revision.Aug 28 2019, 5:24 PM
asomers updated this revision to Diff 61629.Sep 3 2019, 10:30 PM

Add one more change that I forgot to save before starting the review

emaste accepted this revision.Sep 6 2019, 2:31 PM
emaste added a subscriber: emaste.
emaste added inline comments.
tests/sys/fs/fusefs/opendir.cc
124 ↗(On Diff #61629)

Aside, I find this order ( 0 < fd ) odd, but unrelated to this change and the file style should be consistent anyhow.

tests/sys/fs/fusefs/read.cc
116 ↗(On Diff #61629)

I think this will be fine but am not sure it will convince Coverity to drop the warning.

tests/sys/fs/fusefs/write.cc
1218 ↗(On Diff #61629)

why empty space for some but not all?

This revision is now accepted and ready to land.Sep 6 2019, 2:31 PM
asomers added inline comments.Sep 6 2019, 7:47 PM
tests/sys/fs/fusefs/write.cc
1218 ↗(On Diff #61629)

Uh, because I wasn't careful? I'll try to use the space everywhere, and add it before I commit.

This revision was automatically updated to reflect the committed changes.