Skip to content

Conversation

@jsquyres
Copy link
Member

@jsquyres jsquyres commented Aug 1, 2018

This assert() was added in 13ddcbf (which was a back-ported cherry
pick from master: 35438ae), but it is actually exactly the
opposite assert that should be there. The assert() is actually
superflouous, because the "if" statement resulting in invoking this
function is exactly what the assert() should be. Hence, the simplest
solution here is just to remove that assert().

This assert() is not present on master: it's only on the v2.x branch.
Hence, this is not a cherry-pick from master -- it's a direct PR to
the v2.x branch.

Signed-off-by: Jeff Squyres jsquyres@cisco.com

This issue found in Cisco MTT runs -- see https://mtt.open-mpi.org/index.php?do_redir=2657

@jsquyres jsquyres added the bug label Aug 1, 2018
@jsquyres jsquyres added this to the v2.1.4 milestone Aug 1, 2018
@jsquyres jsquyres requested review from bosilca and hppritcha August 1, 2018 20:16
@jsquyres
Copy link
Member Author

jsquyres commented Aug 1, 2018

@hppritcha As cited in the description, this is not a cherry pick because the issue is specific to the v2.x branch (i.e., the issue was introduced when master commit 35438ae was backported / cherry-picked to v2.x and committed in 13ddcbf).

@jsquyres jsquyres changed the title errhandler_predefined: remove erroneous assert() v2.x: errhandler_predefined: remove erroneous assert() Aug 1, 2018
This assert() was added in 13ddcbf (which was a back-ported cherry
pick from master: 35438ae), but it is actually exactly the
opposite assertion that should be there.  Additionally, the assert()
is superflouous, because the "if" statement resulting in invoking this
function is exactly what the assert() should be.  Hence, the simplest
solution here is just to remove that assert().

Note that this assert() is not present on master: it's only on the
v2.x branch.  Hence, this is not a cherry-pick from master -- it's a
direct PR to the v2.x branch.

Signed-off-by: Jeff Squyres <jsquyres@cisco.com>
@jsquyres jsquyres force-pushed the pr/v2.x/fix-predefined-errhandler-backend-fatal-aggregate-assert branch from bc79c3c to fe4ee5c Compare August 1, 2018 20:21
@hppritcha
Copy link
Member

@jsquyres please merge after CI

@jsquyres jsquyres merged commit 526a6a1 into open-mpi:v2.x Aug 1, 2018
@jsquyres jsquyres deleted the pr/v2.x/fix-predefined-errhandler-backend-fatal-aggregate-assert branch August 1, 2018 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants