Merge pull request #12 from spiiroin/mer1005_enable_extra_warnings

Enable extra compilation warnings

configure.ac

@@ -23,7 +23,7 @@ AC_DEFINE_UNQUOTED([PRG_VERSION],[$VERSION],[Version for libiphb])
 # FIXME: DSME_LOG_ENABLE should be proper&working configure option
 #AC_DEFINE([DSME_LOG_ENABLE], [1])
 
-AC_SUBST(C_GENFLAGS, ["-pthread -g -std=c99 -Wall -Wwrite-strings -Wmissing-prototypes -Werror"])
+AC_SUBST(C_GENFLAGS, ["-pthread -g -std=c99 -Wall -Wextra -Wwrite-strings -Wmissing-prototypes -Werror"])
 AC_SUBST(C_OPTFLAGS, ["-O2 -s"])
 AC_SUBST(C_DBGFLAGS, ["-g -DDEBUG -DDSME_LOG_ENABLE"])
 

src/Makefile.am

@@ -1,7 +1,7 @@
 #
 # Generic options
 #
-AM_CFLAGS = -Wall -Wmissing-prototypes -std=c99 -Os -g -fPIC
+AM_CFLAGS = -Wall -Wextra -Wmissing-prototypes -std=c99 -Os -g -fPIC
 AM_CPPFLAGS = -D_GNU_SOURCE
 AM_LDFLAGS = -g -lrt -Wl,--as-needed
 

src/libiphb.c

@@ -32,6 +32,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <limits.h>
 #include <errno.h>
 #include <time.h>
 #include <sys/types.h>
@@ -88,7 +89,7 @@ int
 iphb_I_woke_up(iphb_t iphbh)
 {
   int st;
-  struct _iphb_req_t  req = {IPHB_WAIT};
+  struct _iphb_req_t  req = { .cmd = IPHB_WAIT, };
 
   if (!iphbh) {
     errno = EINVAL;
@@ -168,16 +169,20 @@ iphb_get_fd(iphb_t iphbh)
 time_t
 iphb_wait2(iphb_t iphbh, unsigned mintime, unsigned maxtime, int must_wait, int resume)
 {
-  struct _iphb_req_t  req = {IPHB_WAIT};
-  struct _iphb_wait_resp_t resp = {0};
+  /* Assume failure */
+  time_t result = -1;
 
+  /* Sanity check arguments */
   if( !iphbh || mintime > maxtime ) {
     errno = EINVAL;
-    return (time_t)-1;
+    goto EXIT;
   }
 
+  /* Clear any pending wakeups we might have available */
   (void)suck_data(HB_INST(iphbh)->fd);
 
+  struct _iphb_req_t  req = { .cmd = IPHB_WAIT, };
+
   /* There are apps that contain out of date libiphb versions built
    * in to the application binaries and we need to at least attempt
    * not to break handling of iphb requests that used to be ok.
@@ -206,46 +211,104 @@ iphb_wait2(iphb_t iphbh, unsigned mintime, unsigned maxtime, int must_wait, int
   /* The server side ignores this unless version >= 1 */
   req.u.wait.wakeup     = (resume != 0);
 
-  if (send(HB_INST(iphbh)->fd, &req, sizeof(req), MSG_DONTWAIT|MSG_NOSIGNAL) <= 0)
-    return (time_t)-1;
+  int rc = send(HB_INST(iphbh)->fd, &req, sizeof req,
+                MSG_DONTWAIT|MSG_NOSIGNAL);
 
-  if (!must_wait)
-    return (time_t)0;
+  if( rc == -1 ) {
+    /* Use errno from send() */
+    goto EXIT;
+  }
 
-  fd_set 		readfds;
-  struct timeval        timeout;
-  struct timespec       ts_then;
-  struct timespec       ts_now;
-  int    		st;
+  if( !must_wait ) {
+    /* Request succesfully sent */
+    result = 0;
+    goto EXIT;
+  }
 
+  /* Get current time at start of wait */
+  struct timespec ts = { 0, 0 };
+  clock_gettime(CLOCK_MONOTONIC, &ts);
+
+  time_t t_beg = ts.tv_sec;
+
+  for( ;; ) {
+    fd_set         readfds;
+    struct timeval timeout;
+
+    time_t t_now = ts.tv_sec;
+
+    /* We know time_t is signed. Assume it is integer and
+     * cpu uses two's complement arithmetics, i.e.
+     *
+     * new_timestamp - old_timestamp of signed N-bit values
+     * gives valid unsigned delta of N bits.
+     *
+     * Further assume that an unsigned int can hold at least
+     * 32 bits of that delta, which should keep us from
+     * hitting numerical overflows as long as wait times
+     * stay under 68 years or so.
+     */
+    unsigned waited = (unsigned)(t_now - t_beg);
+
+    if( waited >= maxtime ) {
+      /* Make it look like we got wakeup from server
+       * after the specified wait time is up.
+       *
+       * Note: The server side wakeups are likely to be sent
+       *       anyway - which might cause extra wakeups if an
+       *       input watch / similar is used for this socket.
+       */
+      result = (time_t)waited;
+      goto EXIT;
+    }
 
-  (void)clock_gettime(CLOCK_MONOTONIC, &ts_then);
+    /* Assume time_t can hold INT_MAX and cap individual
+     * wait-for-input timeouts to that */
+    unsigned waittime = maxtime - waited;
 
-  timeout.tv_sec = maxtime;
-  timeout.tv_usec = 0;
+    if( waittime < INT_MAX )
+      timeout.tv_sec  = (time_t)waittime;
+    else
+      timeout.tv_sec  = (time_t)INT_MAX;
+    timeout.tv_usec = 0;
 
-  for (;;) {
     FD_ZERO(&readfds);
     FD_SET(HB_INST(iphbh)->fd, &readfds);
-    st = select(HB_INST(iphbh)->fd + 1, &readfds, NULL, NULL, &timeout);
 
-    (void)clock_gettime(CLOCK_MONOTONIC, &ts_now);
+    rc = select(HB_INST(iphbh)->fd + 1, &readfds, NULL, NULL, &timeout);
+
+    if( rc > 0 ) {
+      struct _iphb_wait_resp_t resp = { .waited = 0, };
+
+      /* Got input, result is: wakeup or error */
+      rc = recv(HB_INST(iphbh)->fd, &resp, sizeof resp, MSG_WAITALL);
 
-    if (st == -1 && errno == EINTR) {
-	if (ts_now.tv_sec - ts_then.tv_sec < maxtime) {
-	  timeout.tv_sec = maxtime - (ts_now.tv_sec - ts_then.tv_sec);
-	  continue;  
-	}
+      if( rc == (ssize_t)sizeof resp ) {
+        /* Wakeup succesfully read */
+        result = resp.waited;
+      }
+      else if( rc >= 0 ) {
+        /* Unexpected EOF / partial read */
+        errno = EIO;
+      }
+      else {
+        /* Use errno from recv() */
+      }
+
+      goto EXIT;
     }
-    break;
+
+    if( rc == -1 && errno != EINTR ) {
+      /* Use errno from select() */
+      goto EXIT;
+    }
+
+    /* Update current time and try again */
+    clock_gettime(CLOCK_MONOTONIC, &ts);
   }
-  if (st == 0) /* timeout */
-    return ts_now.tv_sec - ts_then.tv_sec;
 
-  if (recv(HB_INST(iphbh)->fd, &resp, sizeof(resp), MSG_WAITALL) > 0)
-    return resp.waited;
-  else
-    return (time_t)-1;
+EXIT:
+  return result;
 }
 
 time_t
@@ -270,7 +333,7 @@ iphb_discard_wakeups(iphb_t iphbh)
 
 int iphb_get_stats(iphb_t iphbh, struct iphb_stats *stats)
 {
-  struct _iphb_req_t  req = {IPHB_STAT};
+  struct _iphb_req_t  req = { .cmd = IPHB_STAT, };
   int bytes = -1;
 
   if (!iphbh) {

tests/Makefile.am

@@ -1,7 +1,7 @@
 #
 # Generic options
 #
-AM_CFLAGS = -Wall -Wmissing-prototypes -std=c99 -Os -g -fPIC
+AM_CFLAGS = -Wall -Wextra -Wmissing-prototypes -std=c99 -Os -g -fPIC
 AM_CPPFLAGS = -D_GNU_SOURCE
 AM_LDFLAGS = -g -lrt -Wl,--as-needed
 

tests/hbtest3.c

@@ -1150,20 +1150,23 @@ static void ranges_test(int *xc)
 /** Issue cpu keepalive at start of wakeup */
 static void keepalive_start_cb(hbtimer_t *self)
 {
+  (void)self;
   log_debug("@ %s()", __FUNCTION__);
   xmce_cpu_keepalive_start();
 }
 
 /** ... renew it periodically */
 static void keepalive_renew_cb(hbtimer_t *self)
 {
+  (void)self;
   log_debug("@ %s()", __FUNCTION__);
   xmce_cpu_keepalive_start();
 }
 
 /** ... and terminate before going back to sleep */
 static void keepalive_stop_cb(hbtimer_t *self)
 {
+  (void)self;
   log_debug("@ %s()", __FUNCTION__);
   xmce_cpu_keepalive_stop();
 }