Skip to content

Conversation

@jsquyres
Copy link
Member

@jsquyres jsquyres commented Jul 9, 2019

Unfortunately, #6797 was merged
before all feedback was received (39b799d). This PR is a minor
addendum to that commit.

This PR simply removes a meaningless = {0} operation.

The use of gethostname() here -- and many other places in the code
base -- is technically unsafe. See
#6801 for a further description
of the issue and a suggested fix. But the risk is quite low;
real-world hostnames are usually much shorter than
OPAL_MAXHOSTNAMELEN. Hence, this PR just removes the meaningless
operation and leaves a real fix for gethostname() usage to a potential
future PR.

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

FYI @orivej

Unfortunately, open-mpi#6797 was merged
before all feedback was received (39b799d).  This PR is a minor
addendum to that commit.

This PR simply removes a meaningless `= {0}` operation.

The use of gethostname() here -- and many other places in the code
base -- is technically unsafe.  See
open-mpi#6801 for a further description
of the issue and a suggested fix.  But the risk is quite low;
real-world hostnames are usually much shorter than
OPAL_MAXHOSTNAMELEN.  Hence, this PR just removes the meaningless
operation and leaves a real fix for gethostname() usage to a potential
future PR.

Signed-off-by: Jeff Squyres <[email protected]>
Copy link
Contributor

@orivej orivej left a comment

Choose a reason for hiding this comment

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

Thank you! Let's discuss the generic solution in #6801.

@jsquyres jsquyres merged commit 07c722e into open-mpi:master Jul 9, 2019
@jsquyres jsquyres deleted the pr/trivial-gethostname-unification branch July 9, 2019 19:06
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