-
Notifications
You must be signed in to change notification settings - Fork 61
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
Fixed fabtests issues with the change of cq/eq_read return value #194
Conversation
Signed-off-by: Shantonu Hossain <shantonu.hossain@intel.com>
if (ret == -FI_EAVAIL) { | ||
if (ret == -FI_EAGAIN) | ||
continue; | ||
else if (ret == -FI_EAVAIL) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To follow the coding guidelines, if either portion of an if statement uses braces, both portions should. I.e. rewrite as
if (ret == -FI_EAGAIN) {
continue;
} else ...
Problem repeated throughout patch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about that! Fixing it.
Signed-off-by: Shantonu Hossain <shantonu.hossain@intel.com>
f2c3406
to
58d2b86
Compare
Signed-off-by: Shantonu Hossain <shantonu.hossain@intel.com>
58d2b86
to
f255f0d
Compare
Pushed all the suggested changes! |
@@ -379,7 +383,9 @@ static int shutdown_events(void) | |||
|
|||
while (disconnects_left && !ret) { | |||
ret = fi_eq_sread(eq, &event, &entry, sizeof entry, -1, 0); | |||
if (ret < 0) { | |||
if (ret == -FI_EAGAIN) | |||
continue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will break out of the while loop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually in both loops above, we're blocking forever until we read an event. So even seeing EAGAIN would be unexpected. Both changes above should probably be reverted.
Signed-off-by: Shantonu Hossain <shantonu.hossain@intel.com>
c02d856
to
a43a934
Compare
Fixed fabtests issues with the change of cq/eq_read return value
Fixes #192 @patrickmacarthur @bturrubiates @jithinjosepkl, can you please review?
Signed-off-by: Shantonu Hossain shantonu.hossain@intel.com