Skip to content

Conversation

@solarkennedy
Copy link
Contributor

This adds support to spectatord let systemd know when it is ready.

This is better than just a "fire and forget" type of thing where systemd can't really be sure if spectatord is actually ready to take traffic

https://www.freedesktop.org/software/systemd/man/latest/sd_notify.html#

util/systemd.h Outdated
Comment on lines 125 to 135
// Handle abstract namespace sockets (starting with '@')
if (notify_socket[0] == '@') {
addr.sun_path[0] = '\0';
std::strncpy(addr.sun_path + 1, notify_socket + 1, sizeof(addr.sun_path) - 2);
} else {
std::strncpy(addr.sun_path, notify_socket, sizeof(addr.sun_path) - 1);
}

// Send the notification
ssize_t result = sendto(fd, state.c_str(), state.size(), MSG_NOSIGNAL,
reinterpret_cast<struct sockaddr*>(&addr), sizeof(addr));

Choose a reason for hiding this comment

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

is sizeof(addr) correct? I think it will include padding after the null terminator. from https://github.com/systemd/systemd/blob/51190631968f2a69acf5da3e3412b003805538f2/src/basic/socket-util.c#L1387-L1396, we can see the correct way. adapted for the fact that we already know the length:

Suggested change
// Handle abstract namespace sockets (starting with '@')
if (notify_socket[0] == '@') {
addr.sun_path[0] = '\0';
std::strncpy(addr.sun_path + 1, notify_socket + 1, sizeof(addr.sun_path) - 2);
} else {
std::strncpy(addr.sun_path, notify_socket, sizeof(addr.sun_path) - 1);
}
// Send the notification
ssize_t result = sendto(fd, state.c_str(), state.size(), MSG_NOSIGNAL,
reinterpret_cast<struct sockaddr*>(&addr), sizeof(addr));
socklen_t addr_len;
// Handle abstract namespace sockets (starting with '@')
if (notify_socket[0] == '@') {
addr.sun_path[0] = '\0';
std::memcpy(addr.sun_path + 1, notify_socket + 1, sizeof(addr.sun_path) - 1);
// abstract sockets treated as byte array, not c string
addr_len = offsetof(struct sockaddr_un, sun_path) + notify_socket_len;
} else {
std::memcpy(addr.sun_path, notify_socket, sizeof(addr.sun_path));
addr.sun_path[str_len] = '\0';
// abstract sockets treated as c string. +1 for null termination
addr_len = offsetof(struct sockaddr_un, sun_path) + notify_socket_len + 1;
}
// Send the notification
ssize_t result = sendto(fd, state.c_str(), state.size(), MSG_NOSIGNAL,
reinterpret_cast<struct sockaddr*>(&addr), addr_len);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

classic

This adds support to spectatord let systemd know when it is ready.

This is better than just a "fire and forget" type of thing where systemd
can't really be sure if spectatord is actually ready to take traffic

https://www.freedesktop.org/software/systemd/man/latest/sd_notify.html#
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