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

test/mon: silence warnings from -Wreorder #15692

Merged
merged 1 commit into from Jun 15, 2017

Conversation

Projects
None yet
5 participants
@joscollin
Member

joscollin commented Jun 14, 2017

The following warning appears during make:

ceph/src/test/mon/test-mon-msg.cc: In constructor ‘MonMsgTest::MonMsgTest()’:
ceph/src/test/mon/test-mon-msg.cc:222:12: warning: ‘MonMsgTest::reply_msg’ will be initialized after [-Wreorder]
   Message *reply_msg;
            ^~~~~~~~~
ceph/src/test/mon/test-mon-msg.cc:229:16: warning:   base ‘MonClientHelper’ [-Wreorder]
     lock("lock") { }
                ^
ceph/src/test/mon/test-mon-msg.cc:226:3: warning:   when initialized here [-Wreorder]
   MonMsgTest() :

Signed-off-by: Jos Collin jcollin@redhat.com

@joscollin joscollin added the cleanup label Jun 14, 2017

MonClientHelper(g_ceph_context),
reply_msg(NULL),
lock("lock") { }

This comment has been minimized.

@adamemerson

adamemerson Jun 14, 2017

Contributor

I think it's easier in the long run if, for things like initializing pointers to null and the like, we just use a default value for the class member.

(With that convention things that SHOULD be initialized obviously are or aren't, and reorder takes care of itself.)

This comment has been minimized.

@joscollin

joscollin Jun 15, 2017

Member

I thought about the same, but I didn't give more importance as reply_msg was already used in the member initializer list. http://en.cppreference.com/w/cpp/language/initializer_list.

Fixed as per your review comment. Please verify now. Thanks.

test/mon: silence warnings from -Wreorder
The following warning appears during make:

ceph/src/test/mon/test-mon-msg.cc: In constructor ‘MonMsgTest::MonMsgTest()’:
ceph/src/test/mon/test-mon-msg.cc:222:12: warning: ‘MonMsgTest::reply_msg’ will be initialized after [-Wreorder]
   Message *reply_msg;
            ^~~~~~~~~
ceph/src/test/mon/test-mon-msg.cc:229:16: warning:   base ‘MonClientHelper’ [-Wreorder]
     lock("lock") { }
                ^
ceph/src/test/mon/test-mon-msg.cc:226:3: warning:   when initialized here [-Wreorder]
   MonMsgTest() :

Signed-off-by: Jos Collin <jcollin@redhat.com>
@adamemerson

This comment has been minimized.

Contributor

adamemerson commented Jun 15, 2017

Lugubrious Gerbils Treat Mournfully

@badone badone added the tests label Jun 15, 2017

@tchaikov tchaikov merged commit 954f193 into ceph:master Jun 15, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
arm64 make check arm64 make check succeeded
Details
make check make check succeeded
Details
@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 15, 2017

@jecluis do you think it's a good idea to add this test to "make check"? currently, this test is neither exercised by it nor by any of our test suites.

@joscollin joscollin deleted the joscollin:wip-test-mon-Wreorder-warning branch Jun 15, 2017

@jecluis

This comment has been minimized.

Member

jecluis commented Jun 15, 2017

@tchaikov the test is, unfortunately, quite small. It barely covers a significant portion of messages. I'm guessing that's why it's not being exercised (but don't recall tbh). I'd love to see the coverage increased and having it added to the checks or teuthology though.

I've just created http://tracker.ceph.com/issues/20311 to track this.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 15, 2017

@jecluis thanks, gotcha!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment