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

msg/async/Event: fix valgrind issue init "owner" to zero #10300

Closed
wants to merge 1 commit into from

Conversation

yuyuyu101
Copy link
Member

Fixes: http://tracker.ceph.com/issues/16685
Signed-off-by: Haomai Wang haomai@xsky.com

@@ -113,7 +113,7 @@ class EventCenter {
CephContext *cct;
int nevent;
// Used only to external event
pthread_t owner;
pthread_t owner = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conditional jump or move depends on uninitialised value(s)

the EventCenter should have been bound with its worker thread at that moment, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YES

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so when in_thread() gets called, owner is always initialized. and how come we have this valgrind warning?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I don't know now....

On Fri, Jul 15, 2016 at 1:41 PM, Kefu Chai notifications@github.com wrote:

In src/msg/async/Event.h
#10300 (comment):

@@ -113,7 +113,7 @@ class EventCenter {
CephContext *cct;
int nevent;
// Used only to external event

  • pthread_t owner;
  • pthread_t owner = 0;

so when in_thread() gets called, owner is always initialized. and how
come we have this valgrind warning?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/ceph/ceph/pull/10300/files/0c221a95d729d779af4525618d112a9d9a0242db#r70924114,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA72YVZ6cui-lEtwQ1c3pBR-aGdgRnQXks5qVx2KgaJpZM4JMpEe
.

Best Regards,

Wheat

@tchaikov
Copy link
Contributor

could rerun the failed tests to verify your fix, but i doubt it.

@yuyuyu101 yuyuyu101 closed this Aug 17, 2016
@yuyuyu101 yuyuyu101 deleted the wip-16685 branch August 17, 2016 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants