core: Switch usbi_transfer to store timeout as timespec

The transfer timeout is structured around time values provided by the
clock_gettime() function. This function uses a timespec structure, but
the usbi_transfer structure was storing its calculated timeout in a
timeval structure. This mismatch introduces extra work when checking for
transfer timeouts as there must be a conversion between these two
structures. Eliminate this by storing the calculated timeout as a
timespec, thus allowing direct comparison.

Note that a conversion to a timeval is still necessary in the
libusb_get_next_timeout() function because the public API uses a timeval
structure, but this is now the only place where such a conversion is
done.

Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>

diff --git a/libusb/io.c b/libusb/io.c
index a691311377f787640be66b63195ccccde759fa9b..1068f9f509a1e14832d32516d930dbb9b5b484bb 100644 (file)
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1202,31 +1202,28 @@ void usbi_io_exit(struct libusb_context *ctx)
 static int calculate_timeout(struct usbi_transfer *itransfer)
 {
        int r;
-       struct timespec current_time;
        unsigned int timeout =
                USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer)->timeout;
 
        if (!timeout) {
-               timerclear(&itransfer->timeout);
+               TIMESPEC_CLEAR(&itransfer->timeout);
                return 0;
        }
 
-       r = usbi_backend.clock_gettime(USBI_CLOCK_MONOTONIC, &current_time);
+       r = usbi_backend.clock_gettime(USBI_CLOCK_MONOTONIC, &itransfer->timeout);
        if (r < 0) {
                usbi_err(ITRANSFER_CTX(itransfer),
                        "failed to read monotonic clock, errno=%d", errno);
                return r;
        }
 
-       current_time.tv_sec += timeout / 1000;
-       current_time.tv_nsec += (timeout % 1000) * 1000000;
-
-       while (current_time.tv_nsec >= 1000000000) {
-               current_time.tv_nsec -= 1000000000;
-               current_time.tv_sec++;
+       itransfer->timeout.tv_sec += timeout / 1000U;
+       itransfer->timeout.tv_nsec += (timeout % 1000U) * 1000000L;
+       if (itransfer->timeout.tv_nsec >= 1000000000L) {
+               ++itransfer->timeout.tv_sec;
+               itransfer->timeout.tv_nsec -= 1000000000L;
        }
 
-       TIMESPEC_TO_TIMEVAL(&itransfer->timeout, &current_time);
        return 0;
 }
 
@@ -1345,19 +1342,19 @@ static int arm_timerfd_for_next_timeout(struct libusb_context *ctx)
        struct usbi_transfer *itransfer;
 
        list_for_each_entry(itransfer, &ctx->flying_transfers, list, struct usbi_transfer) {
-               struct timeval *cur_tv = &itransfer->timeout;
+               struct timespec *cur_ts = &itransfer->timeout;
 
                /* if we've reached transfers of infinite timeout, then we have no
                 * arming to do */
-               if (!timerisset(cur_tv))
+               if (!TIMESPEC_IS_SET(cur_ts))
                        goto disarm;
 
                /* act on first transfer that has not already been handled */
                if (!(itransfer->timeout_flags & (USBI_TRANSFER_TIMEOUT_HANDLED | USBI_TRANSFER_OS_HANDLES_TIMEOUT))) {
                        int r;
                        const struct itimerspec it = { {0, 0},
-                               { cur_tv->tv_sec, cur_tv->tv_usec * 1000 } };
-                       usbi_dbg("next timeout originally %dms", USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer)->timeout);
+                               { cur_ts->tv_sec, cur_ts->tv_nsec } };
+                       usbi_dbg("next timeout originally %ums", USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer)->timeout);
                        r = timerfd_settime(ctx->timerfd, TFD_TIMER_ABSTIME, &it, NULL);
                        if (r < 0)
                                return LIBUSB_ERROR_OTHER;
@@ -1382,7 +1379,7 @@ static int arm_timerfd_for_next_timeout(struct libusb_context *ctx)
 static int add_to_flying_list(struct usbi_transfer *itransfer)
 {
        struct usbi_transfer *cur;
-       struct timeval *timeout = &itransfer->timeout;
+       struct timespec *timeout = &itransfer->timeout;
        struct libusb_context *ctx = ITRANSFER_CTX(itransfer);
        int r;
        int first = 1;
@@ -1398,7 +1395,7 @@ static int add_to_flying_list(struct usbi_transfer *itransfer)
        }
 
        /* if we have infinite timeout, append to end of list */
-       if (!timerisset(timeout)) {
+       if (!TIMESPEC_IS_SET(timeout)) {
                list_add_tail(&itransfer->list, &ctx->flying_transfers);
                /* first is irrelevant in this case */
                goto out;
@@ -1407,11 +1404,9 @@ static int add_to_flying_list(struct usbi_transfer *itransfer)
        /* otherwise, find appropriate place in list */
        list_for_each_entry(cur, &ctx->flying_transfers, list, struct usbi_transfer) {
                /* find first timeout that occurs after the transfer in question */
-               struct timeval *cur_tv = &cur->timeout;
+               struct timespec *cur_ts = &cur->timeout;
 
-               if (!timerisset(cur_tv) || (cur_tv->tv_sec > timeout->tv_sec) ||
-                               (cur_tv->tv_sec == timeout->tv_sec &&
-                                       cur_tv->tv_usec > timeout->tv_usec)) {
+               if (!TIMESPEC_IS_SET(cur_ts) || TIMESPEC_CMP(cur_ts, timeout, >)) {
                        list_add_tail(&itransfer->list, &cur->list);
                        goto out;
                }
@@ -1423,12 +1418,12 @@ static int add_to_flying_list(struct usbi_transfer *itransfer)
        list_add_tail(&itransfer->list, &ctx->flying_transfers);
 out:
 #ifdef HAVE_TIMERFD
-       if (first && usbi_using_timerfd(ctx) && timerisset(timeout)) {
+       if (first && usbi_using_timerfd(ctx) && TIMESPEC_IS_SET(timeout)) {
                /* if this transfer has the lowest timeout of all active transfers,
                 * rearm the timerfd with this transfer's timeout */
                const struct itimerspec it = { {0, 0},
-                       { timeout->tv_sec, timeout->tv_usec * 1000 } };
-               usbi_dbg("arm timerfd for timeout in %dms (first in line)",
+                       { timeout->tv_sec, timeout->tv_nsec } };
+               usbi_dbg("arm timerfd for timeout in %ums (first in line)",
                        USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer)->timeout);
                r = timerfd_settime(ctx->timerfd, TFD_TIMER_ABSTIME, &it, NULL);
                if (r < 0) {
@@ -1457,7 +1452,7 @@ static int remove_from_flying_list(struct usbi_transfer *itransfer)
        int r = 0;
 
        usbi_mutex_lock(&ctx->flying_transfers_lock);
-       rearm_timerfd = (timerisset(&itransfer->timeout) &&
+       rearm_timerfd = (TIMESPEC_IS_SET(&itransfer->timeout) &&
                list_first_entry(&ctx->flying_transfers, struct usbi_transfer, list) == itransfer);
        list_del(&itransfer->list);
        if (usbi_using_timerfd(ctx) && rearm_timerfd)
@@ -2022,27 +2017,24 @@ static void handle_timeout(struct usbi_transfer *itransfer)
 static int handle_timeouts_locked(struct libusb_context *ctx)
 {
        int r;
-       struct timespec systime_ts;
-       struct timeval systime;
+       struct timespec systime;
        struct usbi_transfer *itransfer;
 
        if (list_empty(&ctx->flying_transfers))
                return 0;
 
        /* get current time */
-       r = usbi_backend.clock_gettime(USBI_CLOCK_MONOTONIC, &systime_ts);
+       r = usbi_backend.clock_gettime(USBI_CLOCK_MONOTONIC, &systime);
        if (r < 0)
                return r;
 
-       TIMESPEC_TO_TIMEVAL(&systime, &systime_ts);
-
        /* iterate through flying transfers list, finding all transfers that
         * have expired timeouts */
        list_for_each_entry(itransfer, &ctx->flying_transfers, list, struct usbi_transfer) {
-               struct timeval *cur_tv = &itransfer->timeout;
+               struct timespec *cur_ts = &itransfer->timeout;
 
                /* if we've reached transfers of infinite timeout, we're all done */
-               if (!timerisset(cur_tv))
+               if (!TIMESPEC_IS_SET(cur_ts))
                        return 0;
 
                /* ignore timeouts we've already handled */
@@ -2050,9 +2042,7 @@ static int handle_timeouts_locked(struct libusb_context *ctx)
                        continue;
 
                /* if transfer has non-expired timeout, nothing more to do */
-               if ((cur_tv->tv_sec > systime.tv_sec) ||
-                               (cur_tv->tv_sec == systime.tv_sec &&
-                                       cur_tv->tv_usec > systime.tv_usec))
+               if (TIMESPEC_CMP(cur_ts, &systime, >))
                        return 0;
 
                /* otherwise, we've got an expired timeout to handle */
@@ -2600,9 +2590,8 @@ int API_EXPORTED libusb_get_next_timeout(libusb_context *ctx,
        struct timeval *tv)
 {
        struct usbi_transfer *itransfer;
-       struct timespec cur_ts;
-       struct timeval cur_tv;
-       struct timeval next_timeout = { 0, 0 };
+       struct timespec systime;
+       struct timespec next_timeout = { 0, 0 };
        int r;
 
        ctx = usbi_get_context(ctx);
@@ -2622,7 +2611,7 @@ int API_EXPORTED libusb_get_next_timeout(libusb_context *ctx,
                        continue;
 
                /* if we've reached transfers of infinte timeout, we're done looking */
-               if (!timerisset(&itransfer->timeout))
+               if (!TIMESPEC_IS_SET(&itransfer->timeout))
                        break;
 
                next_timeout = itransfer->timeout;
@@ -2630,23 +2619,23 @@ int API_EXPORTED libusb_get_next_timeout(libusb_context *ctx,
        }
        usbi_mutex_unlock(&ctx->flying_transfers_lock);
 
-       if (!timerisset(&next_timeout)) {
+       if (!TIMESPEC_IS_SET(&next_timeout)) {
                usbi_dbg("no URB with timeout or all handled by OS; no timeout!");
                return 0;
        }
 
-       r = usbi_backend.clock_gettime(USBI_CLOCK_MONOTONIC, &cur_ts);
+       r = usbi_backend.clock_gettime(USBI_CLOCK_MONOTONIC, &systime);
        if (r < 0) {
                usbi_err(ctx, "failed to read monotonic clock, errno=%d", errno);
                return 0;
        }
-       TIMESPEC_TO_TIMEVAL(&cur_tv, &cur_ts);
 
-       if (!timercmp(&cur_tv, &next_timeout, <)) {
+       if (!TIMESPEC_CMP(&systime, &next_timeout, <)) {
                usbi_dbg("first timeout already expired");
                timerclear(tv);
        } else {
-               timersub(&next_timeout, &cur_tv, tv);
+               TIMESPEC_SUB(&next_timeout, &systime, &next_timeout);
+               TIMESPEC_TO_TIMEVAL(tv, &next_timeout);
                usbi_dbg("next timeout in %ld.%06lds", (long)tv->tv_sec, (long)tv->tv_usec);
        }
 

diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index c60b20131941abbeddeee39b26efa9813977be70..da9334fa24cc006bcd39bfeca9d36d09b6dc6693 100644 (file)
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -207,7 +207,21 @@ static inline void *usbi_reallocf(void *ptr, size_t size)
        return ret;
 }
 
-#define TIMESPEC_IS_SET(ts) ((ts)->tv_sec != 0 || (ts)->tv_nsec != 0)
+#define TIMESPEC_IS_SET(ts)    ((ts)->tv_sec || (ts)->tv_nsec)
+#define TIMESPEC_CLEAR(ts)     (ts)->tv_sec = (ts)->tv_nsec = 0
+#define TIMESPEC_CMP(a, b, CMP)                                        \
+       (((a)->tv_sec == (b)->tv_sec)                                   \
+        ? ((a)->tv_nsec CMP (b)->tv_nsec)                              \
+        : ((a)->tv_sec CMP (b)->tv_sec))
+#define TIMESPEC_SUB(a, b, result)                                     \
+       do {                                                            \
+               (result)->tv_sec = (a)->tv_sec - (b)->tv_sec;           \
+               (result)->tv_nsec = (a)->tv_nsec - (b)->tv_nsec;        \
+               if ((result)->tv_nsec < 0L) {                           \
+                       --(result)->tv_sec;                             \
+                       (result)->tv_nsec += 1000000000L;               \
+               }                                                       \
+       } while (0)
 
 #if defined(OS_WINDOWS)
 #define TIMEVAL_TV_SEC_TYPE    long
@@ -459,7 +473,7 @@ struct usbi_transfer {
        int num_iso_packets;
        struct list_head list;
        struct list_head completed_list;
-       struct timeval timeout;
+       struct timespec timeout;
        int transferred;
        uint32_t stream_id;
        uint32_t state_flags;   /* Protected by usbi_transfer->lock */

diff --git a/libusb/os/poll_windows.h b/libusb/os/poll_windows.h
index a38e1dad9b8456f9130dab79e1f40f301d02f854..df1781b1dc7b2e3f1e4735df43c764419b99c054 100644 (file)
--- a/libusb/os/poll_windows.h
+++ b/libusb/os/poll_windows.h
@@ -57,19 +57,4 @@ ssize_t usbi_write(int fd, const void *buf, size_t count);
 ssize_t usbi_read(int fd, void *buf, size_t count);
 int usbi_close(int fd);
 
-/*
- * Timeval operations
- */
-#if !defined(timersub)
-#define timersub(a, b, result)                                         \
-       do {                                                            \
-               (result)->tv_sec = (a)->tv_sec - (b)->tv_sec;           \
-               (result)->tv_usec = (a)->tv_usec - (b)->tv_usec;        \
-               if ((result)->tv_usec < 0L) {                           \
-                       --(result)->tv_sec;                             \
-                       (result)->tv_usec += 1000000L;                  \
-               }                                                       \
-       } while (0)
-#endif
-
 #endif

diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index a1366d4f9e2f31f36ff05fa49dd9dc8d59fbe764..b39f8b13e3e374a8f390aba16ad830f9cd205e26 100644 (file)
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11461
+#define LIBUSB_NANO 11462