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

rc: use /etc/hostname if available #585

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

rc: use /etc/hostname if available #585

wants to merge 1 commit into from

Conversation

vapier
Copy link
Member

@vapier vapier commented Jan 23, 2023

This is significantly less overhead than an init script.

This is significantly less overhead than an init script.
Copy link
Contributor

@dwfreed dwfreed left a comment

Choose a reason for hiding this comment

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

This does not handle the case where the hostname is being set via /etc/conf.d/hostname which the initscript allows. A deprecation warning should be added to the initscript, and a louder one should be added for users of the conf.d method.


if (rc_getfile(RC_SYSCONFDIR "/hostname", &buffer, &len)) {
if (buffer[len - 2] == '\n')
buffer[--len - 1] = '\0';
Copy link
Contributor

Choose a reason for hiding this comment

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

Please split this into 2 lines; it'd be much more readable that way.

size_t len;

if (rc_getfile(RC_SYSCONFDIR "/hostname", &buffer, &len)) {
if (buffer[len - 2] == '\n')
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't handle the case where len is 0 or 1, resulting in an out of bounds read and potentially an out of bounds write (!!!)

char *buffer = NULL;
size_t len;

if (rc_getfile(RC_SYSCONFDIR "/hostname", &buffer, &len)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The hostname tool from net-tools on Linux has a -b flag that will fall back to setting the hostname to localhost if the file does not exist or is empty; this would probably be a good idea to implement, as on Linux, the hostname could be an empty string otherwise. hostname also allows for comment lines (line starts with #) in /etc/hostname, which are ignored; this could also be useful.

williamh added a commit that referenced this pull request Jul 26, 2024
This is an alternative to #585 in which we keep the init script but use
`hostname -F` to read the value from @SYSCONFDIR@/hostname. This allows
for comments in the hostname file.

Also, we deprecate using /etc/conf.d/* to set the host name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants