Skip to content

Commit

Permalink
tree-wide: add a single version of "static const int one = 1"
Browse files Browse the repository at this point in the history
All over the place we define local variables for the various sockopts
that take a bool-like "int" value. Sometimes they are const, sometimes
static, sometimes both, sometimes neither.

Let's clean this up, introduce a common const variable "const_int_one"
(as well as one matching "const_int_zero") and use it everywhere, all
acorss the codebase.
  • Loading branch information
poettering committed Oct 15, 2018
1 parent 8e8132c commit 6d5e65f
Show file tree
Hide file tree
Showing 24 changed files with 104 additions and 128 deletions.
14 changes: 5 additions & 9 deletions src/basic/socket-label.c
Expand Up @@ -35,7 +35,7 @@ int socket_address_listen(

_cleanup_close_ int fd = -1;
const char *p;
int r, one;
int r;

assert(a);

Expand Down Expand Up @@ -74,26 +74,22 @@ int socket_address_listen(
return -errno;

if (reuse_port) {
one = 1;
if (setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &one, sizeof(one)) < 0)
if (setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &const_int_one, sizeof(const_int_one)) < 0)
log_warning_errno(errno, "SO_REUSEPORT failed: %m");
}

if (free_bind) {
one = 1;
if (setsockopt(fd, IPPROTO_IP, IP_FREEBIND, &one, sizeof(one)) < 0)
if (setsockopt(fd, IPPROTO_IP, IP_FREEBIND, &const_int_one, sizeof(const_int_one)) < 0)
log_warning_errno(errno, "IP_FREEBIND failed: %m");
}

if (transparent) {
one = 1;
if (setsockopt(fd, IPPROTO_IP, IP_TRANSPARENT, &one, sizeof(one)) < 0)
if (setsockopt(fd, IPPROTO_IP, IP_TRANSPARENT, &const_int_one, sizeof(const_int_one)) < 0)
log_warning_errno(errno, "IP_TRANSPARENT failed: %m");
}
}

one = 1;
if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)) < 0)
if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &const_int_one, sizeof(const_int_one)) < 0)
return -errno;

p = socket_address_get_path(a);
Expand Down
3 changes: 3 additions & 0 deletions src/basic/util.c
Expand Up @@ -51,6 +51,9 @@ int saved_argc = 0;
char **saved_argv = NULL;
static int saved_in_initrd = -1;

const int const_int_zero = 0;
const int const_int_one = 1;

size_t page_size(void) {
static thread_local size_t pgsz = 0;
long r;
Expand Down
3 changes: 3 additions & 0 deletions src/basic/util.h
Expand Up @@ -232,3 +232,6 @@ int version(void);
int str_verscmp(const char *s1, const char *s2);

void disable_coredumps(void);

extern const int const_int_zero;
extern const int const_int_one;
3 changes: 1 addition & 2 deletions src/core/manager.c
Expand Up @@ -873,7 +873,6 @@ static int manager_setup_notify(Manager *m) {
if (m->notify_fd < 0) {
_cleanup_close_ int fd = -1;
union sockaddr_union sa = {};
static const int one = 1;
int salen;

/* First free all secondary fields */
Expand Down Expand Up @@ -901,7 +900,7 @@ static int manager_setup_notify(Manager *m) {
if (r < 0)
return log_error_errno(errno, "bind(%s) failed: %m", m->notify_socket);

r = setsockopt(fd, SOL_SOCKET, SO_PASSCRED, &one, sizeof(one));
r = setsockopt(fd, SOL_SOCKET, SO_PASSCRED, &const_int_one, sizeof(const_int_one));
if (r < 0)
return log_error_errno(errno, "SO_PASSCRED failed: %m");

Expand Down
18 changes: 6 additions & 12 deletions src/core/socket.c
Expand Up @@ -1018,8 +1018,7 @@ static void socket_apply_socket_options(Socket *s, int fd) {
assert(fd >= 0);

if (s->keep_alive) {
int one = 1;
if (setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, &one, sizeof(one)) < 0)
if (setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, &const_int_one, sizeof(const_int_one)) < 0)
log_unit_warning_errno(UNIT(s), errno, "SO_KEEPALIVE failed: %m");
}

Expand Down Expand Up @@ -1048,32 +1047,27 @@ static void socket_apply_socket_options(Socket *s, int fd) {
}

if (s->no_delay) {
int one = 1;

if (s->socket_protocol == IPPROTO_SCTP) {
if (setsockopt(fd, SOL_SCTP, SCTP_NODELAY, &one, sizeof(one)) < 0)
if (setsockopt(fd, SOL_SCTP, SCTP_NODELAY, &const_int_one, sizeof(const_int_one)) < 0)
log_unit_warning_errno(UNIT(s), errno, "SCTP_NODELAY failed: %m");
} else {
if (setsockopt(fd, SOL_TCP, TCP_NODELAY, &one, sizeof(one)) < 0)
if (setsockopt(fd, SOL_TCP, TCP_NODELAY, &const_int_one, sizeof(const_int_one)) < 0)
log_unit_warning_errno(UNIT(s), errno, "TCP_NODELAY failed: %m");
}
}

if (s->broadcast) {
int one = 1;
if (setsockopt(fd, SOL_SOCKET, SO_BROADCAST, &one, sizeof(one)) < 0)
if (setsockopt(fd, SOL_SOCKET, SO_BROADCAST, &const_int_one, sizeof(const_int_one)) < 0)
log_unit_warning_errno(UNIT(s), errno, "SO_BROADCAST failed: %m");
}

if (s->pass_cred) {
int one = 1;
if (setsockopt(fd, SOL_SOCKET, SO_PASSCRED, &one, sizeof(one)) < 0)
if (setsockopt(fd, SOL_SOCKET, SO_PASSCRED, &const_int_one, sizeof(const_int_one)) < 0)
log_unit_warning_errno(UNIT(s), errno, "SO_PASSCRED failed: %m");
}

if (s->pass_sec) {
int one = 1;
if (setsockopt(fd, SOL_SOCKET, SO_PASSSEC, &one, sizeof(one)) < 0)
if (setsockopt(fd, SOL_SOCKET, SO_PASSSEC, &const_int_one, sizeof(const_int_one)) < 0)
log_unit_warning_errno(UNIT(s), errno, "SO_PASSSEC failed: %m");
}

Expand Down
3 changes: 1 addition & 2 deletions src/import/importd.c
Expand Up @@ -584,7 +584,6 @@ static int manager_new(Manager **ret) {
.un.sun_family = AF_UNIX,
.un.sun_path = "/run/systemd/import/notify",
};
static const int one = 1;
int r;

assert(ret);
Expand Down Expand Up @@ -613,7 +612,7 @@ static int manager_new(Manager **ret) {
if (bind(m->notify_fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)) < 0)
return -errno;

if (setsockopt(m->notify_fd, SOL_SOCKET, SO_PASSCRED, &one, sizeof(one)) < 0)
if (setsockopt(m->notify_fd, SOL_SOCKET, SO_PASSCRED, &const_int_one, sizeof(const_int_one)) < 0)
return -errno;

r = sd_event_add_io(m->event, &m->notify_event_source, m->notify_fd, EPOLLIN, manager_on_notify, m);
Expand Down
3 changes: 1 addition & 2 deletions src/journal/journald-audit.c
Expand Up @@ -497,7 +497,6 @@ static int enable_audit(int fd, bool b) {
}

int server_open_audit(Server *s) {
static const int one = 1;
int r;

if (s->audit_fd < 0) {
Expand Down Expand Up @@ -528,7 +527,7 @@ int server_open_audit(Server *s) {
} else
(void) fd_nonblock(s->audit_fd, true);

r = setsockopt(s->audit_fd, SOL_SOCKET, SO_PASSCRED, &one, sizeof(one));
r = setsockopt(s->audit_fd, SOL_SOCKET, SO_PASSCRED, &const_int_one, sizeof(const_int_one));
if (r < 0)
return log_error_errno(errno, "Failed to set SO_PASSCRED on audit socket: %m");

Expand Down
7 changes: 3 additions & 4 deletions src/journal/journald-native.c
Expand Up @@ -443,7 +443,6 @@ int server_open_native_socket(Server*s) {
.un.sun_family = AF_UNIX,
.un.sun_path = "/run/systemd/journal/socket",
};
static const int one = 1;
int r;

assert(s);
Expand All @@ -463,19 +462,19 @@ int server_open_native_socket(Server*s) {
} else
(void) fd_nonblock(s->native_fd, true);

r = setsockopt(s->native_fd, SOL_SOCKET, SO_PASSCRED, &one, sizeof(one));
r = setsockopt(s->native_fd, SOL_SOCKET, SO_PASSCRED, &const_int_one, sizeof(const_int_one));
if (r < 0)
return log_error_errno(errno, "SO_PASSCRED failed: %m");

#if HAVE_SELINUX
if (mac_selinux_use()) {
r = setsockopt(s->native_fd, SOL_SOCKET, SO_PASSSEC, &one, sizeof(one));
r = setsockopt(s->native_fd, SOL_SOCKET, SO_PASSSEC, &const_int_one, sizeof(const_int_one));
if (r < 0)
log_warning_errno(errno, "SO_PASSSEC failed: %m");
}
#endif

r = setsockopt(s->native_fd, SOL_SOCKET, SO_TIMESTAMP, &one, sizeof(one));
r = setsockopt(s->native_fd, SOL_SOCKET, SO_TIMESTAMP, &const_int_one, sizeof(const_int_one));
if (r < 0)
return log_error_errno(errno, "SO_TIMESTAMP failed: %m");

Expand Down
7 changes: 3 additions & 4 deletions src/journal/journald-syslog.c
Expand Up @@ -447,7 +447,6 @@ int server_open_syslog_socket(Server *s) {
.un.sun_family = AF_UNIX,
.un.sun_path = "/run/systemd/journal/dev-log",
};
static const int one = 1;
int r;

assert(s);
Expand All @@ -467,19 +466,19 @@ int server_open_syslog_socket(Server *s) {
} else
(void) fd_nonblock(s->syslog_fd, true);

r = setsockopt(s->syslog_fd, SOL_SOCKET, SO_PASSCRED, &one, sizeof(one));
r = setsockopt(s->syslog_fd, SOL_SOCKET, SO_PASSCRED, &const_int_one, sizeof(const_int_one));
if (r < 0)
return log_error_errno(errno, "SO_PASSCRED failed: %m");

#if HAVE_SELINUX
if (mac_selinux_use()) {
r = setsockopt(s->syslog_fd, SOL_SOCKET, SO_PASSSEC, &one, sizeof(one));
r = setsockopt(s->syslog_fd, SOL_SOCKET, SO_PASSSEC, &const_int_one, sizeof(const_int_one));
if (r < 0)
log_warning_errno(errno, "SO_PASSSEC failed: %m");
}
#endif

r = setsockopt(s->syslog_fd, SOL_SOCKET, SO_TIMESTAMP, &one, sizeof(one));
r = setsockopt(s->syslog_fd, SOL_SOCKET, SO_TIMESTAMP, &const_int_one, sizeof(const_int_one));
if (r < 0)
return log_error_errno(errno, "SO_TIMESTAMP failed: %m");

Expand Down
14 changes: 7 additions & 7 deletions src/libsystemd-network/dhcp-network.c
Expand Up @@ -78,7 +78,7 @@ static int _bind_raw_socket(int ifindex, union sockaddr_union *link,
.filter = filter
};
_cleanup_close_ int s = -1;
int r, on = 1;
int r;

assert(ifindex > 0);
assert(link);
Expand All @@ -87,7 +87,7 @@ static int _bind_raw_socket(int ifindex, union sockaddr_union *link,
if (s < 0)
return -errno;

r = setsockopt(s, SOL_PACKET, PACKET_AUXDATA, &on, sizeof(on));
r = setsockopt(s, SOL_PACKET, PACKET_AUXDATA, &const_int_one, sizeof(const_int_one));
if (r < 0)
return -errno;

Expand Down Expand Up @@ -149,7 +149,7 @@ int dhcp_network_bind_udp_socket(int ifindex, be32_t address, uint16_t port) {
};
_cleanup_close_ int s = -1;
char ifname[IF_NAMESIZE] = "";
int r, on = 1, tos = IPTOS_CLASS_CS6;
int r, tos = IPTOS_CLASS_CS6;

s = socket(AF_INET, SOCK_DGRAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0);
if (s < 0)
Expand All @@ -159,7 +159,7 @@ int dhcp_network_bind_udp_socket(int ifindex, be32_t address, uint16_t port) {
if (r < 0)
return -errno;

r = setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
r = setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &const_int_one, sizeof(const_int_one));
if (r < 0)
return -errno;

Expand All @@ -173,16 +173,16 @@ int dhcp_network_bind_udp_socket(int ifindex, be32_t address, uint16_t port) {
}

if (address == INADDR_ANY) {
r = setsockopt(s, IPPROTO_IP, IP_PKTINFO, &on, sizeof(on));
r = setsockopt(s, IPPROTO_IP, IP_PKTINFO, &const_int_one, sizeof(const_int_one));
if (r < 0)
return -errno;

r = setsockopt(s, SOL_SOCKET, SO_BROADCAST, &on, sizeof(on));
r = setsockopt(s, SOL_SOCKET, SO_BROADCAST, &const_int_one, sizeof(const_int_one));
if (r < 0)
return -errno;

} else {
r = setsockopt(s, IPPROTO_IP, IP_FREEBIND, &on, sizeof(on));
r = setsockopt(s, IPPROTO_IP, IP_FREEBIND, &const_int_one, sizeof(const_int_one));
if (r < 0)
return -errno;
}
Expand Down
8 changes: 4 additions & 4 deletions src/libsystemd-network/dhcp6-network.c
Expand Up @@ -25,7 +25,7 @@ int dhcp6_network_bind_udp_socket(int index, struct in6_addr *local_address) {
.in6.sin6_scope_id = index,
};
_cleanup_close_ int s = -1;
int r, off = 0, on = 1;
int r;

assert(index > 0);
assert(local_address);
Expand All @@ -36,15 +36,15 @@ int dhcp6_network_bind_udp_socket(int index, struct in6_addr *local_address) {
if (s < 0)
return -errno;

r = setsockopt(s, IPPROTO_IPV6, IPV6_V6ONLY, &on, sizeof(on));
r = setsockopt(s, IPPROTO_IPV6, IPV6_V6ONLY, &const_int_one, sizeof(const_int_one));
if (r < 0)
return -errno;

r = setsockopt(s, IPPROTO_IPV6, IPV6_MULTICAST_LOOP, &off, sizeof(off));
r = setsockopt(s, IPPROTO_IPV6, IPV6_MULTICAST_LOOP, &const_int_zero, sizeof(const_int_zero));
if (r < 0)
return -errno;

r = setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
r = setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &const_int_one, sizeof(const_int_one));
if (r < 0)
return -errno;

Expand Down
8 changes: 4 additions & 4 deletions src/libsystemd-network/icmp6-util.c
Expand Up @@ -33,7 +33,7 @@ static int icmp6_bind_router_message(const struct icmp6_filter *filter,
int index = mreq->ipv6mr_interface;
_cleanup_close_ int s = -1;
char ifname[IF_NAMESIZE] = "";
static const int zero = 0, one = 1, hops = 255;
static const int hops = 255;
int r;

s = socket(AF_INET6, SOCK_RAW | SOCK_CLOEXEC | SOCK_NONBLOCK, IPPROTO_ICMPV6);
Expand All @@ -56,7 +56,7 @@ static int icmp6_bind_router_message(const struct icmp6_filter *filter,
if (r < 0)
return -errno;

r = setsockopt(s, IPPROTO_IPV6, IPV6_MULTICAST_LOOP, &zero, sizeof(zero));
r = setsockopt(s, IPPROTO_IPV6, IPV6_MULTICAST_LOOP, &const_int_zero, sizeof(const_int_zero));
if (r < 0)
return -errno;

Expand All @@ -68,11 +68,11 @@ static int icmp6_bind_router_message(const struct icmp6_filter *filter,
if (r < 0)
return -errno;

r = setsockopt(s, SOL_IPV6, IPV6_RECVHOPLIMIT, &one, sizeof(one));
r = setsockopt(s, SOL_IPV6, IPV6_RECVHOPLIMIT, &const_int_one, sizeof(const_int_one));
if (r < 0)
return -errno;

r = setsockopt(s, SOL_SOCKET, SO_TIMESTAMP, &one, sizeof(one));
r = setsockopt(s, SOL_SOCKET, SO_TIMESTAMP, &const_int_one, sizeof(const_int_one));
if (r < 0)
return -errno;

Expand Down
4 changes: 2 additions & 2 deletions src/libsystemd/sd-netlink/netlink-socket.c
Expand Up @@ -88,9 +88,9 @@ static int broadcast_groups_get(sd_netlink *nl) {

int socket_bind(sd_netlink *nl) {
socklen_t addrlen;
int r, one = 1;
int r;

r = setsockopt(nl->fd, SOL_NETLINK, NETLINK_PKTINFO, &one, sizeof(one));
r = setsockopt(nl->fd, SOL_NETLINK, NETLINK_PKTINFO, &const_int_one, sizeof(const_int_one));
if (r < 0)
return -errno;

Expand Down
3 changes: 1 addition & 2 deletions src/libudev/libudev-monitor.c
Expand Up @@ -337,7 +337,6 @@ int udev_monitor_allow_unicast_sender(struct udev_monitor *udev_monitor, struct
* Returns: 0 on success, otherwise a negative error value.
*/
_public_ int udev_monitor_enable_receiving(struct udev_monitor *udev_monitor) {
const int on = 1;
int r;

assert_return(udev_monitor, -EINVAL);
Expand All @@ -358,7 +357,7 @@ _public_ int udev_monitor_enable_receiving(struct udev_monitor *udev_monitor) {
return log_debug_errno(r, "Failed to set address: %m");

/* enable receiving of sender credentials */
if (setsockopt(udev_monitor->sock, SOL_SOCKET, SO_PASSCRED, &on, sizeof(on)) < 0)
if (setsockopt(udev_monitor->sock, SOL_SOCKET, SO_PASSCRED, &const_int_one, sizeof(const_int_one)) < 0)
return log_debug_errno(errno, "Failed to set socket option SO_PASSCRED: %m");

return 0;
Expand Down
3 changes: 1 addition & 2 deletions src/nspawn/nspawn.c
Expand Up @@ -2812,7 +2812,6 @@ static int inner_child(
}

static int setup_sd_notify_child(void) {
static const int one = 1;
int fd = -1;
union sockaddr_union sa = {
.un.sun_family = AF_UNIX,
Expand All @@ -2839,7 +2838,7 @@ static int setup_sd_notify_child(void) {
return log_error_errno(r, "Failed to chown " NSPAWN_NOTIFY_SOCKET_PATH ": %m");
}

r = setsockopt(fd, SOL_SOCKET, SO_PASSCRED, &one, sizeof(one));
r = setsockopt(fd, SOL_SOCKET, SO_PASSCRED, &const_int_one, sizeof(const_int_one));
if (r < 0) {
safe_close(fd);
return log_error_errno(errno, "SO_PASSCRED failed: %m");
Expand Down

6 comments on commit 6d5e65f

@heftig
Copy link
Contributor

@heftig heftig commented on 6d5e65f Oct 17, 2018

Choose a reason for hiding this comment

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

Wouldn't &(int){1}, sizeof(int) be less verbose than &const_int_one, sizeof(const_int_one)?

@heftig
Copy link
Contributor

@heftig heftig commented on 6d5e65f Oct 17, 2018

Choose a reason for hiding this comment

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

Or, as it's used only for setsockopt, a wrapper:

static inline int setsockopt_int(int socket, int level, int option_name, int option_value) {
        return setsockopt(socket, level, option_name, &option_value, sizeof option_value);
}
r = setsockopt_int(fd_worker, SOL_SOCKET, SO_PASSCRED, 1);

@poettering
Copy link
Member Author

Choose a reason for hiding this comment

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

@heftig interesting idea, would be happy to merge such a patch ;-)

@heftig
Copy link
Contributor

@heftig heftig commented on 6d5e65f Oct 18, 2018

Choose a reason for hiding this comment

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

Sure, which one?

@poettering
Copy link
Member Author

Choose a reason for hiding this comment

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

@heftig ah, eh, I decided to cook something along those lines myself, let me finish that quickly.

@poettering
Copy link
Member Author

Choose a reason for hiding this comment

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

@heftig ptal on #10456, it implements your second idea

Please sign in to comment.