Skip to content

Conversation

@jsquyres
Copy link
Member

Don't strcmp against the default value -- the default value may change
over time. Instead, check to see if the MCA var source is not
DEFAULT.

Signed-off-by: Jeff Squyres [email protected]

Don't strcmp against the default value -- the default value may change
over time.  Instead, check to see if the MCA var source is not
DEFAULT.

Signed-off-by: Jeff Squyres <[email protected]>
@jsquyres
Copy link
Member Author

@rhc54 I'm not 100% sure this is the right robustification.

This check simply checks to ensure that the value was set by someone other than the default. But what if someone set it to exactly the same value as the default value?

I guess my question is: what exactly do we need to test here?

@rhc54
Copy link
Contributor

rhc54 commented Aug 19, 2016

I think this is exactly what we need - if someone "sets" it to the default value, then they have "confirmed" that this is what they want and we should obey it. If they specified nothing, then we want to open the door to look for alternatives.

So I'd say this is fine and should be committed - can then PR it across to 2.0.1

@jsquyres
Copy link
Member Author

@rhc54 +1 I'll merge.

I think this can wait until v2.0.2 -- unless you strongly feel it needs to be in v2.0.1? I.e., I think your existing code is more-or-less fine; this is a bit stronger enforcement, but not a critical issue. Thoughts?

@jsquyres jsquyres merged commit bb6b87f into open-mpi:master Aug 19, 2016
@jsquyres jsquyres deleted the pr/rsh-robustify-default-check branch August 19, 2016 17:33
@rhc54
Copy link
Contributor

rhc54 commented Aug 19, 2016

agreed, it can wait

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.

2 participants