Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wip cppcheck errors #14446

Merged
merged 6 commits into from Apr 12, 2017

Conversation

Projects
None yet
2 participants
@badone
Copy link
Contributor

badone commented Apr 11, 2017

Various fixes for the cppcheck errors seen http://people.redhat.com/bhubbard/cppcheck/cppcheck.17-04-05.txt

badone added some commits Apr 11, 2017

blkdev: Suppress cppcheck error
Signed-off-by: Brad Hubbard <bhubbard@redhat.com>
crypto: Suppress cppcheck error
Signed-off-by: Brad Hubbard <bhubbard@redhat.com>
common: Restructure cppcheck suppresions
Some of the suppressions are currently redundant and some need to be
changed.

Signed-off-by: Brad Hubbard <bhubbard@redhat.com>
denc: Silence cppcheck uninitialised variable errors
Signed-off-by: Brad Hubbard <bhubbard@redhat.com>
test: Silence cppcheck memory leak warnings in ceph_test_rados_api_io
Signed-off-by: Brad Hubbard <bhubbard@redhat.com>

@badone badone requested a review from tchaikov Apr 11, 2017

@tchaikov
Copy link
Contributor

tchaikov left a comment

aside from the nits, lgtm.

delete[] buf2;
delete[] buf3;
};
scope_guard<decltype(lam)> sg(std::forward<decltype(lam)>(lam));

This comment has been minimized.

Copy link
@tchaikov

tchaikov Apr 11, 2017

Contributor

why not use make_scope_guard()?

delete[] buf2;
delete[] buf3;
};
scope_guard<decltype(lam)> sg(std::forward<decltype(lam)>(lam));

This comment has been minimized.

Copy link
@tchaikov

tchaikov Apr 11, 2017

Contributor

why not use make_scope_guard()?

This comment has been minimized.

Copy link
@badone

badone Apr 11, 2017

Author Contributor

Yes, good question. The reason is that cppcheck issues an error about a memory leak (false positive) when make_scope_guard is used but does not when creating a scope_guard object with a named lambda. I have reported this upstream and am trying to get access to create a cppcheck issue.

This comment has been minimized.

Copy link
@tchaikov

tchaikov Apr 12, 2017

Contributor

but i don't think we should trade the readability for appeasing the static analyzer if the code is correct. moreover, we can avoid use names like lam (i figured out it's the short for lambda just now).

@@ -826,6 +827,12 @@ TEST_F(LibRadosIoEC, OverlappingWriteRoundTrip) {
char *buf = (char *)new char[dbsize];
char *buf2 = (char *)new char[bsize];
char *buf3 = (char *)new char[dbsize];
auto lam = [&] {

This comment has been minimized.

Copy link
@tchaikov

tchaikov Apr 11, 2017

Contributor

how about just put

char buf[dbsize];
char buf2[bsize];
char buf3[dbsize];

?

This comment has been minimized.

Copy link
@badone

badone Apr 11, 2017

Author Contributor

Well, yes, we could change everything to use the stack, that would be easy, but I figured I would follow the lead set by the original author.

This comment has been minimized.

Copy link
@tchaikov

tchaikov Apr 12, 2017

Contributor

i was wrong, C++ (at this moment) does not support VLA. but GCC does.

@badone

This comment has been minimized.

Copy link
Contributor Author

badone commented Apr 11, 2017

Error EIO: load dlopen(/home/jenkins-build/build/workspace/ceph-pull-requests/build/lib/libec_FAKE.so): /home/jenkins-build/build/workspace/ceph-pull-requests/build/lib/libec_FAKE.so: cannot open shared object file: No such file or directory

Checking if the above is transient.

Jenkins test this please.

@badone badone force-pushed the badone:wip-cppcheck-errors branch from 12e1b24 to 9b182c8 Apr 12, 2017

delete[] buf3;
delete[] unalignedbuf;
};
scope_guard<decltype(cleanup)> sg(std::forward<decltype(cleanup)>(cleanup));

This comment has been minimized.

Copy link
@tchaikov

tchaikov Apr 12, 2017

Contributor

nit, sg(std::move(cleanup)) would suffice. we know the type of cleanup here. and we do want to pass by rvalue reference here.

This comment has been minimized.

Copy link
@badone

badone Apr 12, 2017

Author Contributor

Right, as always, thanks!

test: Suppress cppcheck error
Signed-off-by: Brad Hubbard <bhubbard@redhat.com>

@badone badone force-pushed the badone:wip-cppcheck-errors branch from 9b182c8 to d105ec3 Apr 12, 2017

@badone badone merged commit 6650e3e into ceph:master Apr 12, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@badone badone deleted the badone:wip-cppcheck-errors branch Apr 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.