Skip to content

Conversation

@orivej
Copy link
Contributor

@orivej orivej commented Jul 5, 2019

Previously verbose output of if_linux_ipv6_open looked like this:

found interface ab c: 0ab: a b: abc: 0 0: a 0:abcd: 0 0 scope 0

This changes the output to:

found interface eth0 inet6 ab0c:ab:a0b:abc:0:a00:abcd:0 scope 0

@ompiteam-bot
Copy link

Can one of the admins verify this patch?

@bwbarrett
Copy link
Member

ok to test

@rhc54 rhc54 added this to the v4.0.2 milestone Jul 8, 2019
@orivej orivej force-pushed the fix-if_linux_ipv6-addrstr branch from 366990a to ecd02eb Compare July 8, 2019 20:57
@jsquyres jsquyres force-pushed the fix-if_linux_ipv6-addrstr branch from ecd02eb to 0d2d55b Compare July 8, 2019 22:02
@jsquyres
Copy link
Member

jsquyres commented Jul 8, 2019

@orivej I just did the following:

  1. Squashed your 2 commits down to 1
  2. Added two more small commits (which we should squash before merging):
    1. Use opal_show_help() instead of opal_output(). opal_show_help() de-duplicates error messages, so that you don't see the same error message from every single MPI process.
    2. Fix 2 minor compiler warnings.

I left these 2 additional commits so that you can easily see the changes.

If you're good with them, I can re-squash all down into a single commit and we can merge when CI completes.

Copy link
Contributor Author

@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.

Everything else looks good, thank you!

Previously the verbose output of if_linux_ipv6_open looked like this:

    found interface ab c: 0ab: a b: abc: 0 0: a 0:abcd: 0 0 scope 0

This changes the output to:

    found interface eth0 inet6 ab0c:ab:a0b:abc:0:a00:abcd:0 scope 0

Signed-off-by: Orivej Desh <[email protected]>
Signed-off-by: Jeff Squyres <[email protected]>
@jsquyres jsquyres force-pushed the fix-if_linux_ipv6-addrstr branch from 0d2d55b to 39b799d Compare July 9, 2019 12:46
Copy link
Member

@jsquyres jsquyres left a comment

Choose a reason for hiding this comment

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

I just squashed everything down to one commit.

@orivej You're the commit author, and you should feel free to put in your copyright, if you wish. Let me know if you want to amend again (i.e., before we merge to master).

@hppritcha hppritcha merged commit 29468ec into open-mpi:master Jul 9, 2019
jsquyres added a commit to jsquyres/ompi that referenced this pull request Jul 9, 2019
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]>
@hppritcha hppritcha removed this from the v4.0.2 milestone Jul 11, 2019
@hppritcha
Copy link
Member

this should not had the v4.0.2 milestone set. That's for PRs that are going into v4.0.x. I

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.

6 participants