Merge pull request #379 from naokiiwakami/issue-378

Issue #378 - Have wiringPiISR and wiringPiISR2 return error on failure in initialization

wiringPi/wiringPi.c

@@ -2868,35 +2868,25 @@ int waitForInterruptClose(int pin) {
 }
 
 /*
- * interruptHandlerV2:
- *	This is a thread and gets started to wait for the interrupt we're
- *	hoping to catch. It will call the user-function when the interrupt
- *	fires.
+ * interruptHandlerInit:
+ *	Initializes an interrupt handler before starting the listener loop in a
+ *	separate thread.
+ *	Returns: >0 on successful initialization, 0 if the listener loop does not
+ *	have to start, -1 on error.
  *********************************************************************************
  */
 
-void *interruptHandlerV2(void *arg)
+static int interruptHandlerInit(int pin, int EdgeMode, unsigned long debounce_period_us)
 {
   const char* strmode = "";
-  int pin, EdgeMode, ret, fd, attr, i;
-  unsigned int readret;
-  unsigned long debounce_period_us;
-  struct pollfd polls ;  
+  int ret, attr;
   struct gpio_v2_line_config config;
   struct gpio_v2_line_request req;
-  struct gpio_v2_line_event evdat[64];  
-  struct WPIWfiStatus wfiStatus;
-  struct timespec tspec = {0, 5e5};  /* 0.5 ms timeout {0, 1e6} */
 
-  pin = *(int *)arg;
-
-  if (wiringPiGpioDeviceGetFd()<0) {
-    return NULL;
+  if (wiringPiGpioDeviceGetFd() < 0) {
+    return -1;
   }
 
-  EdgeMode = isrEdgeMode[pin];
-  debounce_period_us = isrDebouncePeriodUs[pin];
- 
   if (wiringPiDebug) {
     printf ("interruptHandlerV2: GPIO line %d, edge mode %d, debounce_period_us %lu \n", pin, EdgeMode, debounce_period_us) ;
   }
@@ -2912,7 +2902,7 @@ void *interruptHandlerV2(void *arg)
       if (wiringPiDebug) {
         printf ("interruptHandlerV2: waitForInterruptMode edge mode INT_EDGE_SETUP - exiting\n") ;
       }
-      return NULL;
+      return 0;
     case INT_EDGE_FALLING:
       config.flags |= GPIO_V2_LINE_FLAG_EDGE_FALLING;
       strmode = "falling";
@@ -2944,14 +2934,44 @@ void *interruptHandlerV2(void *arg)
   ret = ioctl(chipFd, GPIO_V2_GET_LINE_IOCTL, &req);
   if (ret == -1) {
     ReportDeviceError("interruptHandlerV2: get line event", pin , strmode, ret);
-    return NULL;
+    return -1;
   }
 
-  if (wiringPiDebug) 
-    printf ("interruptHandlerV2: GPIO get line %d , mode %s succeded, fd=%d\n", pin, strmode, req.fd) ;
+  if (wiringPiDebug) {
+    printf ("interruptHandlerV2: GPIO get line %d , mode %s succeded, fd=%d\n", pin, strmode, req.fd);
+  }
+
+  return req.fd;
+}
+
+struct interrupt_handler_params {
+  int pin;
+  int fd;
+};
+
+/*
+ * interruptHandlerV2:
+ *	This is a thread and gets started to wait for the interrupt we're
+ *	hoping to catch. It will call the user-function when the interrupt
+ *	fires.
+ *********************************************************************************
+ */
+
+static void *interruptHandlerV2(void *arg)
+{
+  struct interrupt_handler_params *params;
+  int pin, ret, fd, i;
+  unsigned int readret;
+  struct pollfd polls ;
+  struct gpio_v2_line_event evdat[64];
+  struct WPIWfiStatus wfiStatus;
+  struct timespec tspec = {0, 5e5};  /* 0.5 ms timeout {0, 1e6} */
+
+  params = (struct interrupt_handler_params *)arg;
+  pin = params->pin;
+  fd = params->fd;
 
   /* set event fd  */
-  fd = req.fd;
   isrFds [pin] = fd;
 
   (void)piHiPri (55) ;	// Only effective if we run as root
@@ -3041,9 +3061,8 @@ void *interruptHandlerV2(void *arg)
   }
 }
 
-
 /*
- * wiringPiISR:
+ * wiringPiISRInternal:
  *	Pi Specific.
  *	Take the details and create an interrupt handler that will do a call-
  *	back to the user supplied function.
@@ -3064,39 +3083,49 @@ int wiringPiISRInternal(int pin, int edgeMode, void (*function)(struct WPIWfiSta
     printf("wiringPi: wiringPiISR pin %d, edgeMode %d\n", pin, edgeMode);
   }
   if (isrFunctions[pin] || isrFunctionsV2[pin]) {
-    fprintf(stderr, "wiringPi: ISR function already active, ignoring \n");
+    fprintf(stderr, "wiringPi: ISR function already active\n");
   }
 
+  if (wiringPiDebug) {
+    printf("wiringPi: mutex in\n");
+  }
+  pthread_mutex_lock (&pinMutex) ;
+    struct interrupt_handler_params params = {
+      .pin = pin,
+    };
+    params.fd = interruptHandlerInit(pin, edgeMode, debounce_period_us);
+    if (params.fd < 0) {
+      pthread_mutex_unlock (&pinMutex) ;
+      return -1;
+    }
+
+    // OK to start the new ISR. Update the table.
     isrFunctionsV2[pin] = function;
     isrUserdata[pin] = userdata;
     isrFunctions[pin] = functionClassic;
     isrEdgeMode[pin] = edgeMode;
     isrDebouncePeriodUs[pin] = debounce_period_us;
 
-  if (wiringPiDebug) {
-    printf("wiringPi: mutex in\n");
-  }
-  pthread_mutex_lock (&pinMutex) ;
     pinPass = pin ;
+    if (params.fd > 0) {
       if (wiringPiDebug) {
         printf("wiringPi: pthread_create before 0x%lX\n", (unsigned long)isrThreads[pin]);
       }
-    if (pthread_create (&isrThreads[pin], NULL, interruptHandlerV2, &pin)==0) {
+      if (pthread_create (&isrThreads[pin], NULL, interruptHandlerV2, &params)==0) {
         if (wiringPiDebug) {
           printf("wiringPi: pthread_create successed, 0x%lX\n", (unsigned long)isrThreads[pin]);
         }
-/*      while (pinPass != -1)
-        delay (1) ; */
-    // wait so that interruptHandler is up und running. 
-    // when interruptHandler is running, the calling function wiringPiISR
-    // must be still alive, otherwise the thread argument &pin points into nirwana,
-    // when it is picked up from interruptHandler.
-      delay (10);
       } else {
         if (wiringPiDebug) {
           printf("wiringPi: pthread_create failed\n");
         }
       }
+      // wait so that interruptHandler is up und running.
+      // when interruptHandler is running, the calling function wiringPiISRInternal
+      // must be still alive, otherwise the thread argument &param points into nirwana,
+      // when it is picked up from interruptHandlerV2.
+      delay(10);
+    }
 
     if (wiringPiDebug) {
       printf("wiringPi: mutex out\n");