Skip to content
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

Amend #3043 by allowing a longer hostname and simplifying the code. #3044

Merged
merged 1 commit into from Sep 3, 2023

Conversation

susilehtola
Copy link
Member

Description

Improvements discussed in a comment to #3043. The 64-character limit may not be valid on all architectures; this PR should avoid having to revisit the patch.

User API & Changelog headlines

  • RN 1
  • RN 2

Dev notes & details

  • Feature1
  • Feature2

Questions

  • Question1

Checklist

Status

  • Ready for review
  • Ready for merge

// host name has up to HOST_NAME_MAX (64 or 256) + 1 bytes, and must end in the null byte
std::vector<char> name(257);
error = gethostname(name.data(), name.size());
if (error) strncpy(name.data(), "nohostname", name.size());
Copy link
Member

@loriab loriab Sep 3, 2023

Choose a reason for hiding this comment

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

I wasn't sure about 11 -> name.size() here, but https://cplusplus.com/reference/cstring/strncpy/ convinced me that it's doing the padding right.

overall, lgtm. any concerns, @zachglick ?

Copy link
Contributor

Choose a reason for hiding this comment

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

no concerns, I agree this is a better way to do things. thanks Susi!

@loriab loriab added the backport label Sep 3, 2023
@loriab loriab added this to the Psi4 1.9 milestone Sep 3, 2023
@loriab loriab added this pull request to the merge queue Sep 3, 2023
Merged via the queue into psi4:master with commit 97f4680 Sep 3, 2023
5 checks passed
@susilehtola susilehtola deleted the longer_hostname branch September 4, 2023 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants