Skip to content

Conversation

@tastypackets
Copy link

Add sys/kernel/hostname and sys/kernel/domainname support.

Added a new line check on hostname & domainname which are used as a terminator like sysctl. Without the new line terminator trying to clear the values does not work. For example writing "" to domainname is ignored, but writing "\n" does clear the domainname.

Related docs: https://man7.org/linux/man-pages/man5/proc_sys_kernel.5.html

@tastypackets tastypackets force-pushed the feat/add-hostname-domainname branch from f35035e to f233ee7 Compare December 30, 2025 07:43
@tastypackets
Copy link
Author

@eminence Hey would it be possible to get a review on this PR? I was needing hostname and domain for a project I'm working on.

@eminence
Copy link
Owner

eminence commented Jan 4, 2026

Sure thing, I'll try to take a look this weekend

Copy link
Owner

@eminence eminence left a comment

Choose a reason for hiding this comment

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

In general looks fine, but two small comments:

  • Do we really need these two examples? They're not really doing much.
  • Can you add a cfg check in the new tests so that they won't set a hostname unless some feature flag is enabled? In general I don't want changes to be made to the running system (and these tests probably require root to run successfully?)

Comment on lines 619 to 621
let test_domain = String::from("test-host");

let test_domain = "test.local".to_string();
Copy link
Owner

Choose a reason for hiding this comment

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

One of these variables is not needed

Copy link
Author

Choose a reason for hiding this comment

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

Nice catch think I copy pasted something and wasn't paying attention. 😓

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.

2 participants