-
Notifications
You must be signed in to change notification settings - Fork 14
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
handle keys with internal nuls and ignore systemd #8
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but the systemd stuff also needs changes to the unit files, want to just split them into separate PRs?
src/main.rs
Outdated
let listener = unsafe { UnixListener::from_raw_fd(LISTEN_FDS_START) }; | ||
let path = Path::new(SOCKET_PATH); | ||
std::fs::create_dir_all(path.parent().expect("socket path has no parent"))?; | ||
std::fs::remove_file(path).ok(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't let _ =
more idiomatic? (I am not actually sure)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let _ =
what?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think std::fs::remove_file(path).ok()
is equivalent to saying "remove the file if possible, and I expect that if there are any errors with that we should proceed, including the one I expect most which is ENOENT
which is fine, any other errors like permissions will probably surface when we try to bind".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm used to seeing let _ = std::fs::remove_file(path)
. But apparently silencing the warning with .ok()
is also reasonable according to https://stackoverflow.com/questions/51141672/how-do-i-ignore-an-error-returned-from-a-rust-function-and-proceed-regardless
Yeah I intentionally didn't touch the unit files, I sort of wanted you to do that. If you can guide my hand I can do it here. |
Deserialize keys that have nuls in them (because these might be ip addresses), handle nul-terminated strings higher up when we know what kind of request it is. fixes #7
Deserialize keys that have nuls in them (because these might be ip
addresses), handle nul-terminated strings higher up when we know what
kind of request it is.
Don't use systemd socket activation becuase startup ordering is weird.
fixes #7