Bugfix: Problems exposed by unit tests for f_signal.

Fix ordering of functions.
Compare using "== -1" rather than "< 0" because the POSIX standard says "-1" rather than "less than zero".

The f_signal_read() function has several problems:
- Should be passing a pointer rather than an array to poll().
- Needs to check data_poll.revents.
- Missing the parameter checking.

diff --git a/level_0/f_signal/c/signal.c b/level_0/f_signal/c/signal.c
index 54ae284fea50bd0295e3cd48d1e8d011cb4b6876..664d4b322bd34337eb8fdc00f9b913e03d48bb08 100644 (file)
--- a/level_0/f_signal/c/signal.c
+++ b/level_0/f_signal/c/signal.c
@@ -14,7 +14,7 @@ extern "C" {
       return F_data_not;
     }
 
-    if (close(signal->id) < 0) {
+    if (close(signal->id) == -1) {
       if (errno == EBADF) return F_status_set_error(F_descriptor);
       if (errno == EDQUOT) return F_status_set_error(F_filesystem_quota_block);
       if (errno == EINTR) return F_status_set_error(F_interrupt);
@@ -30,6 +30,23 @@ extern "C" {
   }
 #endif // _di_f_signal_close_
 
+#ifndef _di_f_signal_mask_
+  f_status_t f_signal_mask(const int how, const sigset_t * const next, sigset_t * const current) {
+    #ifndef _di_level_0_parameter_checking_
+      if (!next && !current) return F_status_set_error(F_parameter);
+    #endif // _di_level_0_parameter_checking_
+
+    if (sigprocmask(how, next, current) == -1) {
+      if (errno == EFAULT) return F_status_set_error(F_buffer);
+      if (errno == EINVAL) return F_status_set_error(F_parameter);
+
+      return F_status_set_error(F_failure);
+    }
+
+    return F_none;
+  }
+#endif // _di_f_signal_mask_
+
 #ifndef _di_f_signal_open_
   f_status_t f_signal_open(f_signal_t * const signal) {
     #ifndef _di_level_0_parameter_checking_
@@ -38,7 +55,7 @@ extern "C" {
 
     const int result = signalfd(signal->id, &signal->set, signal->flags);
 
-    if (result < 0) {
+    if (result == -1) {
       if (errno == EINVAL) return F_status_set_error(F_parameter);
       if (errno == EMFILE) return F_status_set_error(F_file_descriptor_max);
       if (errno == ENFILE) return F_status_set_error(F_file_open_max);
@@ -54,24 +71,37 @@ extern "C" {
   }
 #endif // _di_f_signal_open_
 
-#ifndef _di_f_signal_read_
-  f_status_t f_signal_read(const f_signal_t signal, const int timeout, struct signalfd_siginfo * const information) {
+#ifndef _di_f_signal_queue_
+  f_status_t f_signal_queue(const pid_t id, const int signal, const union sigval value) {
 
-    if (!signal.id) {
-      return F_data_not;
+    if (sigqueue(id, signal, value) == -1) {
+      if (errno == EAGAIN) return F_status_set_error(F_resource_not);
+      if (errno == ENOSYS) return F_status_set_error(F_supported_not);
+      if (errno == EINVAL) return F_status_set_error(F_parameter);
+      if (errno == ESRCH) return F_status_set_error(F_found_not);
+
+      return F_status_set_error(F_failure);
     }
 
+    return F_none;
+  }
+#endif // _di_f_signal_queue_
+
+#ifndef _di_f_signal_read_
+  f_status_t f_signal_read(const f_signal_t signal, const int timeout, struct signalfd_siginfo * const information) {
+    #ifndef _di_level_0_parameter_checking_
+      if (!information) return F_status_set_error(F_parameter);
+    #endif // _di_level_0_parameter_checking_
+
     struct pollfd data_poll;
     memset(&data_poll, 0, sizeof(struct pollfd));
 
     data_poll.fd = signal.id;
     data_poll.events = POLLIN;
 
-    struct pollfd polls[] = { data_poll };
-
-    const int result = poll(polls, 1, timeout);
+    const int result = poll(&data_poll, 1, timeout);
 
-    if (result < 0) {
+    if (result == -1) {
       if (errno == EFAULT) return F_status_set_error(F_buffer);
       if (errno == EINTR) return F_status_set_error(F_interrupt);
       if (errno == EINVAL) return F_status_set_error(F_parameter);
@@ -79,22 +109,37 @@ extern "C" {
 
       return F_status_set_error(F_failure);
     }
-    else if (result) {
-      const ssize_t total = read(signal.id, information, sizeof(struct signalfd_siginfo));
-
-      if (total < 0) {
-        if (errno == EAGAIN || errno == EWOULDBLOCK) return F_status_set_error(F_block);
-        if (errno == EBADF) return F_status_set_error(F_descriptor);
-        if (errno == EFAULT) return F_status_set_error(F_buffer);
-        if (errno == EINTR) return F_status_set_error(F_interrupt);
-        if (errno == EINVAL) return F_status_set_error(F_parameter);
-        if (errno == EIO) return F_status_set_error(F_input_output);
-        if (errno == EISDIR) return F_status_set_error(F_file_type_directory);
-
-        return F_status_set_error(F_failure);
+
+    if (result) {
+      if (data_poll.revents & POLLNVAL) {
+        return F_status_set_error(F_parameter);
       }
 
-      return F_signal;
+      if (data_poll.revents & POLLHUP) {
+        return F_status_set_error(F_file_closed);
+      }
+
+      if (data_poll.revents & POLLERR) {
+        return F_status_set_error(F_stream);
+      }
+
+      if (data_poll.revents & POLLIN) {
+        const ssize_t total = read(signal.id, information, sizeof(struct signalfd_siginfo));
+
+        if (total == -1) {
+          if (errno == EAGAIN || errno == EWOULDBLOCK) return F_status_set_error(F_block);
+          if (errno == EBADF) return F_status_set_error(F_descriptor);
+          if (errno == EFAULT) return F_status_set_error(F_buffer);
+          if (errno == EINTR) return F_status_set_error(F_interrupt);
+          if (errno == EINVAL) return F_status_set_error(F_parameter);
+          if (errno == EIO) return F_status_set_error(F_input_output);
+          if (errno == EISDIR) return F_status_set_error(F_file_type_directory);
+
+          return F_status_set_error(F_failure);
+        }
+
+        return F_signal;
+      }
     }
 
     return F_none;
@@ -104,7 +149,7 @@ extern "C" {
 #ifndef _di_f_signal_send_
   f_status_t f_signal_send(const int signal, const pid_t process_id) {
 
-    if (kill(process_id, signal) < 0) {
+    if (kill(process_id, signal) == -1) {
       if (errno == EINVAL) return F_status_set_error(F_parameter);
       if (errno == EPERM) return F_status_set_error(F_prohibited);
       if (errno == ESRCH) return F_status_set_error(F_found_not);
@@ -122,7 +167,7 @@ extern "C" {
       if (!set) return F_status_set_error(F_parameter);
     #endif // _di_level_0_parameter_checking_
 
-    if (sigaddset(set, signal) < 0) {
+    if (sigaddset(set, signal) == -1) {
       if (errno == EINVAL) return F_status_set_error(F_parameter);
 
       return F_status_set_error(F_failure);
@@ -138,7 +183,7 @@ extern "C" {
       if (!set) return F_status_set_error(F_parameter);
     #endif // _di_level_0_parameter_checking_
 
-    if (sigdelset(set, signal) < 0) {
+    if (sigdelset(set, signal) == -1) {
       if (errno == EINVAL) return F_status_set_error(F_parameter);
 
       return F_status_set_error(F_failure);
@@ -154,7 +199,7 @@ extern "C" {
       if (!set) return F_status_set_error(F_parameter);
     #endif // _di_level_0_parameter_checking_
 
-    if (sigemptyset(set) < 0) {
+    if (sigemptyset(set) == -1) {
       if (errno == EINVAL) return F_status_set_error(F_parameter);
 
       return F_status_set_error(F_failure);
@@ -170,7 +215,7 @@ extern "C" {
       if (!set) return F_status_set_error(F_parameter);
     #endif // _di_level_0_parameter_checking_
 
-    if (sigfillset(set) < 0) {
+    if (sigfillset(set) == -1) {
       if (errno == EFAULT) return F_status_set_error(F_buffer);
       if (errno == EINVAL) return F_status_set_error(F_parameter);
 
@@ -181,39 +226,6 @@ extern "C" {
   }
 #endif // _di_f_signal_set_fill_
 
-#ifndef _di_f_signal_mask_
-  f_status_t f_signal_mask(const int how, const sigset_t * const next, sigset_t * const current) {
-    #ifndef _di_level_0_parameter_checking_
-      if (!next && !current) return F_status_set_error(F_parameter);
-    #endif // _di_level_0_parameter_checking_
-
-    if (sigprocmask(how, next, current) < 0) {
-      if (errno == EFAULT) return F_status_set_error(F_buffer);
-      if (errno == EINVAL) return F_status_set_error(F_parameter);
-
-      return F_status_set_error(F_failure);
-    }
-
-    return F_none;
-  }
-#endif // _di_f_signal_mask_
-
-#ifndef _di_f_signal_queue_
-  f_status_t f_signal_queue(const pid_t id, const int signal, const union sigval value) {
-
-    if (sigqueue(id, signal, value) < 0) {
-      if (errno == EAGAIN) return F_status_set_error(F_resource_not);
-      if (errno == ENOSYS) return F_status_set_error(F_supported_not);
-      if (errno == EINVAL) return F_status_set_error(F_parameter);
-      if (errno == ESRCH) return F_status_set_error(F_found_not);
-
-      return F_status_set_error(F_failure);
-    }
-
-    return F_none;
-  }
-#endif // _di_f_signal_queue_
-
 #ifndef _di_f_signal_set_has_
   f_status_t f_signal_set_has(const int signal, const sigset_t * const set) {
     #ifndef _di_level_0_parameter_checking_
@@ -222,7 +234,7 @@ extern "C" {
 
     const int result = sigismember(set, signal);
 
-    if (result < 0) {
+    if (result == -1) {
       if (errno == EINVAL) return F_status_set_error(F_parameter);
 
       return F_status_set_error(F_failure);
@@ -235,7 +247,7 @@ extern "C" {
 #ifndef _di_f_signal_wait_
   f_status_t f_signal_wait(const sigset_t * const set, siginfo_t * const information) {
 
-    if (sigwaitinfo(set, information) < 0) {
+    if (sigwaitinfo(set, information) == -1) {
       if (errno == EINTR) return F_status_set_error(F_interrupt);
       if (errno == EINVAL) return F_status_set_error(F_parameter);
 
@@ -249,7 +261,7 @@ extern "C" {
 #ifndef _di_f_signal_wait_until_
   f_status_t f_signal_wait_until(const sigset_t * const set, const struct timespec * timeout, siginfo_t * const information) {
 
-    if (sigtimedwait(set, information, timeout) < 0) {
+    if (sigtimedwait(set, information, timeout) == -1) {
       if (errno == EAGAIN) return F_time_out;
       if (errno == EINTR) return F_status_set_error(F_interrupt);
       if (errno == EINVAL) return F_status_set_error(F_parameter);

diff --git a/level_0/f_signal/c/signal.h b/level_0/f_signal/c/signal.h
index 1f98a8d9987b7fb805e3f4875ca289e4c2476bf8..3d811f6eef092f0d9dd205d1c6d7faddea0d31fd 100644 (file)
--- a/level_0/f_signal/c/signal.h
+++ b/level_0/f_signal/c/signal.h
@@ -54,6 +54,34 @@ extern "C" {
 #endif // _di_f_signal_close_
 
 /**
+ * Get or assign the current signal set in use.
+ *
+ * Either set or previous may be NULL but not both (at least one is required).
+ *
+ * @param how
+ *   How to handle the signal.
+ *   Set this to 0 when only trying to get the current signal set.
+ * @param next
+ *   (optional) The new set of signals to handle.
+ *   Set to NULL to not use.
+ * @param current
+ *   (optional) The current set of signals being handled.
+ *   Set to NULL to not use.
+ *
+ * @return
+ *   F_none on success but no signal found.
+ *
+ *   F_parameter (with error bit) if a parameter is invalid.
+ *
+ *   F_failure (with error bit) for any other error.
+ *
+ * @see sigprocmask()
+ */
+#ifndef _di_f_signal_mask_
+  extern f_status_t f_signal_mask(const int how, const sigset_t * const next, sigset_t * const current);
+#endif // _di_f_signal_mask_
+
+/**
  * Open a signal descriptor, listening for the given set of signals.
  *
  * The signal.id is assigned with the signal descriptor on success.
@@ -66,6 +94,7 @@ extern "C" {
  *
  * @return
  *   F_none on success but no signal found.
+ *
  *   F_device (with error bit) if could not mount the internal inode device.
  *   F_file_descriptor_max (with error bit) if max file descriptors is reached.
  *   F_memory_not (with error bit) if out of memory.
@@ -80,6 +109,32 @@ extern "C" {
 #endif // _di_f_signal_open_
 
 /**
+ * Send the signal and value to the given process.
+ *
+ * @param id
+ *   The PID to signal.
+ * @param signal
+ *   The signal to send to the thread.
+ * @param value
+ *   The signal value to send.
+ *
+ * @return
+ *   F_none on success but no signal found.
+ *
+ *   F_found_not (with error bit) if the given PID was found.
+ *   F_parameter (with error bit) if a parameter is invalid.
+ *   F_resource_not (with error bit) if the max signals is reached.
+ *   F_supported_not (with error bit) if this action is not supported by the current OS.
+ *
+ *   F_failure (with error bit) for any other error.
+ *
+ * @see sigqueue()
+ */
+#ifndef _di_f_signal_queue_
+  extern f_status_t f_signal_queue(const pid_t id, const int signal, const union sigval value);
+#endif // _di_f_signal_queue_
+
+/**
  * Read a current process signal, if one exists.
  *
  * @param signal
@@ -98,9 +153,11 @@ extern "C" {
  *   F_block (with error bit) if file descriptor is set to non-block and the read would result in a blocking operation.
  *   F_buffer (with error bit) if the buffer is invalid.
  *   F_descriptor (with error bit) if the signal descriptor is invalid.
+ *   F_file_closed (with error bit) if the signal descriptor stream is closed.
  *   F_interrupt (with error bit) if interrupt was received.
  *   F_memory_not (with error bit) on out of memory.
  *   F_parameter (with error bit) if a parameter is invalid.
+ *   F_stream (with error bit) when the poll() returns POLLERR (file stream error).
  *   F_input_output (with error bit) on I/O error.
  *   F_file_type_directory (with error bit) if file descriptor represents a directory.
  *
@@ -218,60 +275,6 @@ extern "C" {
 #endif // _di_f_signal_set_fill_
 
 /**
- * Get or assign the current signal set in use.
- *
- * Either set or previous may be NULL but not both (at least one is required).
- *
- * @param how
- *   How to handle the signal.
- *   Set this to 0 when only trying to get the current signal set.
- * @param next
- *   (optional) The new set of signals to handle.
- *   Set to NULL to not use.
- * @param current
- *   (optional) The current set of signals being handled.
- *   Set to NULL to not use.
- *
- * @return
- *   F_none on success but no signal found.
- *
- *   F_parameter (with error bit) if a parameter is invalid.
- *
- *   F_failure (with error bit) for any other error.
- *
- * @see sigprocmask()
- */
-#ifndef _di_f_signal_mask_
-  extern f_status_t f_signal_mask(const int how, const sigset_t * const next, sigset_t * const current);
-#endif // _di_f_signal_mask_
-
-/**
- * Send the signal and value to the given process.
- *
- * @param id
- *   The PID to signal.
- * @param signal
- *   The signal to send to the thread.
- * @param value
- *   The signal value to send.
- *
- * @return
- *   F_none on success but no signal found.
- *
- *   F_found_not (with error bit) if the given PID was found.
- *   F_parameter (with error bit) if a parameter is invalid.
- *   F_resource_not (with error bit) if the max signals is reached.
- *   F_supported_not (with error bit) if this action is not supported by the current OS.
- *
- *   F_failure (with error bit) for any other error.
- *
- * @see sigqueue()
- */
-#ifndef _di_f_signal_queue_
-  extern f_status_t f_signal_queue(const pid_t id, const int signal, const union sigval value);
-#endif // _di_f_signal_queue_
-
-/**
  * Check to see if the given signal set has a given signal.
  *
  * @param signal

diff --git a/level_0/f_signal/c/signal/common.h b/level_0/f_signal/c/signal/common.h
index 8d3d67dcdb0d3f6ca55984ba598878522c9dc027..a61742ac4f1b7e6e8fec826be541f007e2abb077 100644 (file)
--- a/level_0/f_signal/c/signal/common.h
+++ b/level_0/f_signal/c/signal/common.h
@@ -30,7 +30,7 @@ extern "C" {
     sigset_t set;
   } f_signal_t;
 
-  #define f_signal_t_initialize {0, -1, { 0 } }
+  #define f_signal_t_initialize { 0, -1, { 0 } }
 
   #define macro_f_signal_t_initialize(flags, id, set) { flags, id, set }
 

diff --git a/level_0/f_signal/data/build/settings b/level_0/f_signal/data/build/settings
index f10f330fa0477b6d26532e1b7892b2ceeb3f0dca..8765d336e6f1eed6475261e5f73d618eda88eab8 100644 (file)
--- a/level_0/f_signal/data/build/settings
+++ b/level_0/f_signal/data/build/settings
@@ -47,7 +47,7 @@ search_static yes
 
 flags -O2 -z now -g -fdiagnostics-color=always -Wno-logical-not-parentheses -Wno-parentheses
 flags-clang -Wno-logical-op-parentheses
-flags-test -O0 -fstack-protector -Wall
+flags-test -O0 -fstack-protector -Wall -Wno-missing-braces
 flags-coverage -O0 --coverage -fprofile-abs-path -fprofile-dir=build/coverage/
 
 flags_library -fPIC