From ffb5926a30c07be8775c0df046672b427756cf85 Mon Sep 17 00:00:00 2001 From: Chris Dickens Date: Thu, 28 Jan 2016 14:24:09 -0800 Subject: [PATCH] linux_netlink: Add useful debug messages and clean up error handling Also addresses some minor whitespace and stylistic issues. Signed-off-by: Chris Dickens --- libusb/os/linux_netlink.c | 168 +++++++++++++++++++++++++--------------------- libusb/version_nano.h | 2 +- 2 files changed, 91 insertions(+), 79 deletions(-) diff --git a/libusb/os/linux_netlink.c b/libusb/os/linux_netlink.c index 5ad1e62..501a94d 100644 --- a/libusb/os/linux_netlink.c +++ b/libusb/os/linux_netlink.c @@ -22,14 +22,14 @@ #include -#include -#include +#include #include #include #include #include #include #include +#include #include #ifdef HAVE_ASM_TYPES_H @@ -50,7 +50,7 @@ static pthread_t libusb_linux_event_thread; static void *linux_netlink_event_thread_main(void *arg); -static struct sockaddr_nl snl = { .nl_family=AF_NETLINK, .nl_groups=KERNEL }; +static struct sockaddr_nl snl = { .nl_family = AF_NETLINK, .nl_groups = KERNEL }; static int set_fd_cloexec_nb(int fd) { @@ -58,22 +58,30 @@ static int set_fd_cloexec_nb(int fd) #if defined(FD_CLOEXEC) flags = fcntl(fd, F_GETFD); - if (0 > flags) { + if (flags == -1) { + usbi_err(NULL, "failed to get netlink fd flags (%d)", errno); return -1; } if (!(flags & FD_CLOEXEC)) { - fcntl(fd, F_SETFD, flags | FD_CLOEXEC); + if (fcntl(fd, F_SETFD, flags | FD_CLOEXEC) == -1) { + usbi_err(NULL, "failed to set netlink fd flags (%d)", errno); + return -1; + } } #endif flags = fcntl(fd, F_GETFL); - if (0 > flags) { + if (flags == -1) { + usbi_err(NULL, "failed to get netlink fd status flags (%d)", errno); return -1; } if (!(flags & O_NONBLOCK)) { - fcntl(fd, F_SETFL, flags | O_NONBLOCK); + if (fcntl(fd, F_SETFL, flags | O_NONBLOCK) == -1) { + usbi_err(NULL, "failed to set netlink fd status flags (%d)", errno); + return -1; + } } return 0; @@ -84,8 +92,6 @@ int linux_netlink_start_event_monitor(void) int socktype = SOCK_RAW; int ret; - snl.nl_groups = KERNEL; - #if defined(SOCK_CLOEXEC) socktype |= SOCK_CLOEXEC; #endif @@ -94,25 +100,24 @@ int linux_netlink_start_event_monitor(void) #endif linux_netlink_socket = socket(PF_NETLINK, socktype, NETLINK_KOBJECT_UEVENT); - if (-1 == linux_netlink_socket && EINVAL == errno) { + if (linux_netlink_socket == -1 && errno == EINVAL) { + usbi_dbg("failed to create netlink socket of type %d, attempting SOCK_RAW", socktype); linux_netlink_socket = socket(PF_NETLINK, SOCK_RAW, NETLINK_KOBJECT_UEVENT); } - if (-1 == linux_netlink_socket) { - return LIBUSB_ERROR_OTHER; + if (linux_netlink_socket == -1) { + usbi_err(NULL, "failed to create netlink socket (%d)", errno); + goto err; } - ret = set_fd_cloexec_nb (linux_netlink_socket); - if (0 != ret) { - close (linux_netlink_socket); - linux_netlink_socket = -1; - return LIBUSB_ERROR_OTHER; - } + ret = set_fd_cloexec_nb(linux_netlink_socket); + if (ret == -1) + goto err_close_socket; - ret = bind(linux_netlink_socket, (struct sockaddr *) &snl, sizeof(snl)); - if (0 != ret) { - close(linux_netlink_socket); - return LIBUSB_ERROR_OTHER; + ret = bind(linux_netlink_socket, (struct sockaddr *)&snl, sizeof(snl)); + if (ret == -1) { + usbi_err(NULL, "failed to bind netlink socket (%d)", errno); + goto err_close_socket; } /* TODO -- add authentication */ @@ -120,38 +125,43 @@ int linux_netlink_start_event_monitor(void) ret = usbi_pipe(netlink_control_pipe); if (ret) { - usbi_err(NULL, "could not create netlink control pipe"); - close(linux_netlink_socket); - return LIBUSB_ERROR_OTHER; + usbi_err(NULL, "failed to create netlink control pipe"); + goto err_close_socket; } ret = pthread_create(&libusb_linux_event_thread, NULL, linux_netlink_event_thread_main, NULL); - if (0 != ret) { - close(netlink_control_pipe[0]); - close(netlink_control_pipe[1]); - close(linux_netlink_socket); - return LIBUSB_ERROR_OTHER; + if (ret != 0) { + usbi_err(NULL, "failed to create netlink event thread (%d)", ret); + goto err_close_pipe; } return LIBUSB_SUCCESS; + +err_close_pipe: + close(netlink_control_pipe[0]); + close(netlink_control_pipe[1]); + netlink_control_pipe[0] = -1; + netlink_control_pipe[1] = -1; +err_close_socket: + close(linux_netlink_socket); + linux_netlink_socket = -1; +err: + return LIBUSB_ERROR_OTHER; } int linux_netlink_stop_event_monitor(void) { - int r; char dummy = 1; + ssize_t r; - if (-1 == linux_netlink_socket) { - /* already closed. nothing to do */ - return LIBUSB_SUCCESS; - } + assert(linux_netlink_socket != -1); /* Write some dummy data to the control pipe and * wait for the thread to exit */ r = usbi_write(netlink_control_pipe[1], &dummy, sizeof(dummy)); - if (r <= 0) { + if (r <= 0) usbi_warn(NULL, "netlink control pipe signal failed"); - } + pthread_join(libusb_linux_event_thread, NULL); close(linux_netlink_socket); @@ -166,14 +176,14 @@ int linux_netlink_stop_event_monitor(void) return LIBUSB_SUCCESS; } -static const char *netlink_message_parse (const char *buffer, size_t len, const char *key) +static const char *netlink_message_parse(const char *buffer, size_t len, const char *key) { size_t keylen = strlen(key); size_t offset; - for (offset = 0 ; offset < len && '\0' != buffer[offset] ; offset += strlen(buffer + offset) + 1) { - if (0 == strncmp(buffer + offset, key, keylen) && - '=' == buffer[offset + keylen]) { + for (offset = 0; offset < len && buffer[offset] != '\0'; offset += strlen(buffer + offset) + 1) { + if (strncmp(buffer + offset, key, keylen) == 0 && + buffer[offset + keylen] == '=') { return buffer + offset + keylen + 1; } } @@ -182,8 +192,9 @@ static const char *netlink_message_parse (const char *buffer, size_t len, const } /* parse parts of netlink message common to both libudev and the kernel */ -static int linux_netlink_parse(char *buffer, size_t len, int *detached, const char **sys_name, - uint8_t *busnum, uint8_t *devaddr) { +static int linux_netlink_parse(const char *buffer, size_t len, int *detached, + const char **sys_name, uint8_t *busnum, uint8_t *devaddr) +{ const char *tmp; int i; @@ -194,57 +205,56 @@ static int linux_netlink_parse(char *buffer, size_t len, int *detached, const ch *busnum = 0; *devaddr = 0; - tmp = netlink_message_parse((const char *) buffer, len, "ACTION"); - if (tmp == NULL) + tmp = netlink_message_parse(buffer, len, "ACTION"); + if (!tmp) { return -1; - if (0 == strcmp(tmp, "remove")) { + } else if (strcmp(tmp, "remove") == 0) { *detached = 1; - } else if (0 != strcmp(tmp, "add")) { + } else if (strcmp(tmp, "add") != 0) { usbi_dbg("unknown device action %s", tmp); return -1; } /* check that this is a usb message */ tmp = netlink_message_parse(buffer, len, "SUBSYSTEM"); - if (NULL == tmp || 0 != strcmp(tmp, "usb")) { + if (!tmp || strcmp(tmp, "usb") != 0) { /* not usb. ignore */ return -1; } /* check that this is an actual usb device */ tmp = netlink_message_parse(buffer, len, "DEVTYPE"); - if (NULL == tmp || 0 != strcmp(tmp, "usb_device")) { + if (!tmp || strcmp(tmp, "usb_device") != 0) { /* not usb. ignore */ return -1; } tmp = netlink_message_parse(buffer, len, "BUSNUM"); - if (NULL == tmp) { + if (!tmp) { /* no bus number. try "DEVICE" */ tmp = netlink_message_parse(buffer, len, "DEVICE"); - if (NULL == tmp) { + if (!tmp) { /* not usb. ignore */ return -1; } - + /* Parse a device path such as /dev/bus/usb/003/004 */ - char *pLastSlash = (char*)strrchr(tmp,'/'); - if(NULL == pLastSlash) { + const char *pLastSlash = strrchr(tmp, '/'); + if (!pLastSlash) return -1; - } - *devaddr = strtoul(pLastSlash + 1, NULL, 10); + *busnum = (uint8_t)(strtoul(pLastSlash - 3, NULL, 10) & 0xff); if (errno) { errno = 0; return -1; } - - *busnum = strtoul(pLastSlash - 3, NULL, 10); + + *devaddr = (uint8_t)(strtoul(pLastSlash + 1, NULL, 10) & 0xff); if (errno) { errno = 0; return -1; } - + return 0; } @@ -255,9 +265,8 @@ static int linux_netlink_parse(char *buffer, size_t len, int *detached, const ch } tmp = netlink_message_parse(buffer, len, "DEVNUM"); - if (NULL == tmp) { + if (!tmp) return -1; - } *devaddr = (uint8_t)(strtoul(tmp, NULL, 10) & 0xff); if (errno) { @@ -266,12 +275,11 @@ static int linux_netlink_parse(char *buffer, size_t len, int *detached, const ch } tmp = netlink_message_parse(buffer, len, "DEVPATH"); - if (NULL == tmp) { + if (!tmp) return -1; - } - for (i = strlen(tmp) - 1 ; i ; --i) { - if ('/' ==tmp[i]) { + for (i = strlen(tmp) - 1; i; --i) { + if (tmp[i] == '/') { *sys_name = tmp + i + 1; break; } @@ -283,14 +291,16 @@ static int linux_netlink_parse(char *buffer, size_t len, int *detached, const ch static int linux_netlink_read_message(void) { - unsigned char buffer[1024]; - struct iovec iov = {.iov_base = buffer, .iov_len = sizeof(buffer)}; - struct msghdr meh = { .msg_iov=&iov, .msg_iovlen=1, - .msg_name=&snl, .msg_namelen=sizeof(snl) }; + char buffer[1024]; const char *sys_name = NULL; uint8_t busnum, devaddr; int detached, r; - size_t len; + ssize_t len; + struct iovec iov = { .iov_base = buffer, .iov_len = sizeof(buffer) }; + struct msghdr meh = { + .msg_iov = &iov, .msg_iovlen = 1, + .msg_name = &snl, .msg_namelen = sizeof(snl) + }; /* read netlink message */ memset(buffer, 0, sizeof(buffer)); @@ -303,8 +313,7 @@ static int linux_netlink_read_message(void) /* TODO -- authenticate this message is from the kernel or udevd */ - r = linux_netlink_parse(buffer, len, &detached, &sys_name, - &busnum, &devaddr); + r = linux_netlink_parse(buffer, (size_t)len, &detached, &sys_name, &busnum, &devaddr); if (r) return r; @@ -323,7 +332,7 @@ static int linux_netlink_read_message(void) static void *linux_netlink_event_thread_main(void *arg) { char dummy; - int r; + ssize_t r; struct pollfd fds[] = { { .fd = netlink_control_pipe[0], .events = POLLIN }, @@ -333,22 +342,25 @@ static void *linux_netlink_event_thread_main(void *arg) UNUSED(arg); + usbi_dbg("netlink event thread entering"); + while (poll(fds, 2, -1) >= 0) { if (fds[0].revents & POLLIN) { /* activity on control pipe, read the byte and exit */ r = usbi_read(netlink_control_pipe[0], &dummy, sizeof(dummy)); - if (r <= 0) { + if (r <= 0) usbi_warn(NULL, "netlink control pipe read failed"); - } break; } if (fds[1].revents & POLLIN) { - usbi_mutex_static_lock(&linux_hotplug_lock); - linux_netlink_read_message(); - usbi_mutex_static_unlock(&linux_hotplug_lock); + usbi_mutex_static_lock(&linux_hotplug_lock); + linux_netlink_read_message(); + usbi_mutex_static_unlock(&linux_hotplug_lock); } } + usbi_dbg("netlink event thread exiting"); + return NULL; } diff --git a/libusb/version_nano.h b/libusb/version_nano.h index b7a0eb1..f055648 100644 --- a/libusb/version_nano.h +++ b/libusb/version_nano.h @@ -1 +1 @@ -#define LIBUSB_NANO 11050 +#define LIBUSB_NANO 11051 -- 2.7.4