Replace error prone signal() with sigaction()

Up to this date signal() was used which implementation could vary [1].
Sigaction() call is preferred. This commit introduces replacement
from signal() to sigaction() by the use of signal_s() wrapper.
Also remove redundant signal.h header includes.

[1] https://man7.org/linux/man-pages/man2/signal.2.html

Signed-off-by: Lukasz Florczak <lukasz.florczak@linux.intel.com>
Signed-off-by: Jes Sorensen <jsorensen@fb.com>

diff --git a/Grow.c b/Grow.c
index aa72490b8968358604a8d908691704cb5e08779b..18c5719bc1982a20518c1ae4d869601a1f19b7b1 100644 (file)
--- a/Grow.c
+++ b/Grow.c
@@ -26,7 +26,6 @@
 #include       <sys/mman.h>
 #include       <stddef.h>
 #include       <stdint.h>
-#include       <signal.h>
 #include       <sys/wait.h>
 
 #if ! defined(__BIG_ENDIAN) && ! defined(__LITTLE_ENDIAN)
@@ -3566,7 +3565,8 @@ started:
                fd = -1;
        mlockall(MCL_FUTURE);
 
-       signal(SIGTERM, catch_term);
+       if (signal_s(SIGTERM, catch_term) == SIG_ERR)
+               goto release;
 
        if (st->ss->external) {
                /* metadata handler takes it from here */

diff --git a/Monitor.c b/Monitor.c
index 30c031a25fc7d2c2b602dcc88e9773a1aa9487c2..c0ab5412261dbff743cfaf4fc6eeb8e9aad1a56f 100644 (file)
--- a/Monitor.c
+++ b/Monitor.c
@@ -26,7 +26,6 @@
 #include       "md_p.h"
 #include       "md_u.h"
 #include       <sys/wait.h>
-#include       <signal.h>
 #include       <limits.h>
 #include       <syslog.h>
 #ifndef NO_LIBUDEV
@@ -435,8 +434,10 @@ static void alert(char *event, char *dev, char *disc, struct alert_info *info)
                if (mp) {
                        FILE *mdstat;
                        char hname[256];
+
                        gethostname(hname, sizeof(hname));
-                       signal(SIGPIPE, SIG_IGN);
+                       signal_s(SIGPIPE, SIG_IGN);
+
                        if (info->mailfrom)
                                fprintf(mp, "From: %s\n", info->mailfrom);
                        else

diff --git a/managemon.c b/managemon.c
index bb7334cf1b965635dc29addb239ace5b180d7042..0e9bdf00b954bd8a641ea2f9d0c5da43dd5a4df8 100644 (file)
--- a/managemon.c
+++ b/managemon.c
@@ -106,7 +106,6 @@
 #include       "mdmon.h"
 #include       <sys/syscall.h>
 #include       <sys/socket.h>
-#include       <signal.h>
 
 static void close_aa(struct active_array *aa)
 {

diff --git a/mdadm.h b/mdadm.h
index c7268a712c9de3d003902ec333df22046a4d85b3..26e7e5cd97c6a046e0c1d52a5452c022a54baa46 100644 (file)
--- a/mdadm.h
+++ b/mdadm.h
@@ -46,6 +46,7 @@ extern __off64_t lseek64 __P ((int __fd, __off64_t __offset, int __whence));
 #include       <string.h>
 #include       <syslog.h>
 #include       <stdbool.h>
+#include       <signal.h>
 /* Newer glibc requires sys/sysmacros.h directly for makedev() */
 #include       <sys/sysmacros.h>
 #ifdef __dietlibc__
@@ -1729,6 +1730,27 @@ static inline char *to_subarray(struct mdstat_ent *ent, char *container)
        return &ent->metadata_version[10+strlen(container)+1];
 }
 
+/**
+ * signal_s() - Wrapper for sigaction() with signal()-like interface.
+ * @sig: The signal to set the signal handler to.
+ * @handler: The signal handler.
+ *
+ * Return: previous handler or SIG_ERR on failure.
+ */
+static inline sighandler_t signal_s(int sig, sighandler_t handler)
+{
+       struct sigaction new_act;
+       struct sigaction old_act;
+
+       new_act.sa_handler = handler;
+       new_act.sa_flags = 0;
+
+       if (sigaction(sig, &new_act, &old_act) == 0)
+               return old_act.sa_handler;
+
+       return SIG_ERR;
+}
+
 #ifdef DEBUG
 #define dprintf(fmt, arg...) \
        fprintf(stderr, "%s: %s: "fmt, Name, __func__, ##arg)

diff --git a/mdmon.c b/mdmon.c
index c71e62c6b35a7e4d3ed7751794a2b0674b41113f..5570574b4f1a43d1c270812ede80f03fd8affa23 100644 (file)
--- a/mdmon.c
+++ b/mdmon.c
@@ -56,7 +56,6 @@
 #include       <errno.h>
 #include       <string.h>
 #include       <fcntl.h>
-#include       <signal.h>
 #include       <dirent.h>
 #ifdef USE_PTHREADS
 #include       <pthread.h>

diff --git a/monitor.c b/monitor.c
index e0d3be679daf25324db8e0672da16802c9c46b43..b877e595c9989620195e7976d2079a3f9ef05775 100644 (file)
--- a/monitor.c
+++ b/monitor.c
@@ -22,7 +22,6 @@
 #include "mdmon.h"
 #include <sys/syscall.h>
 #include <sys/select.h>
-#include <signal.h>
 
 static char *array_states[] = {
        "clear", "inactive", "suspended", "readonly", "read-auto",

diff --git a/probe_roms.c b/probe_roms.c
index 7ea04c7ade280ba54b30769d0a86be3307a5bbcd..94c80c2cc6d0f24fa4c9a370608f2d130006636b 100644 (file)
--- a/probe_roms.c
+++ b/probe_roms.c
@@ -22,7 +22,6 @@
 #include "probe_roms.h"
 #include "mdadm.h"
 #include <unistd.h>
-#include <signal.h>
 #include <fcntl.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
@@ -69,7 +68,8 @@ static int probe_address16(const __u16 *ptr, __u16 *val)
 
 void probe_roms_exit(void)
 {
-       signal(SIGBUS, SIG_DFL);
+       signal_s(SIGBUS, SIG_DFL);
+
        if (rom_fd >= 0) {
                close(rom_fd);
                rom_fd = -1;
@@ -98,7 +98,7 @@ int probe_roms_init(unsigned long align)
        if (roms_init())
                return -1;
 
-       if (signal(SIGBUS, sigbus) == SIG_ERR)
+       if (signal_s(SIGBUS, sigbus) == SIG_ERR)
                rc = -1;
        if (rc == 0) {
                fd = open("/dev/mem", O_RDONLY);

diff --git a/raid6check.c b/raid6check.c
index a8e6005bc1be5f6936cd4242b0921156578390b3..99477761c640b696bc1a36df10747b5defe54025 100644 (file)
--- a/raid6check.c
+++ b/raid6check.c
@@ -24,7 +24,6 @@
 
 #include "mdadm.h"
 #include <stdint.h>
-#include <signal.h>
 #include <sys/mman.h>
 
 #define CHECK_PAGE_BITS (12)
@@ -130,30 +129,36 @@ void raid6_stats(int *disk, int *results, int raid_disks, int chunk_size)
 }
 
 int lock_stripe(struct mdinfo *info, unsigned long long start,
-               int chunk_size, int data_disks, sighandler_t *sig) {
+               int chunk_size, int data_disks, sighandler_t *sig)
+{
        int rv;
+
+       sig[0] = signal_s(SIGTERM, SIG_IGN);
+       sig[1] = signal_s(SIGINT, SIG_IGN);
+       sig[2] = signal_s(SIGQUIT, SIG_IGN);
+
+       if (sig[0] == SIG_ERR || sig[1] == SIG_ERR || sig[2] == SIG_ERR)
+               return 1;
+
        if(mlockall(MCL_CURRENT | MCL_FUTURE) != 0) {
                return 2;
        }
 
-       sig[0] = signal(SIGTERM, SIG_IGN);
-       sig[1] = signal(SIGINT, SIG_IGN);
-       sig[2] = signal(SIGQUIT, SIG_IGN);
-
        rv = sysfs_set_num(info, NULL, "suspend_lo", start * chunk_size * data_disks);
        rv |= sysfs_set_num(info, NULL, "suspend_hi", (start + 1) * chunk_size * data_disks);
        return rv * 256;
 }
 
-int unlock_all_stripes(struct mdinfo *info, sighandler_t *sig) {
+int unlock_all_stripes(struct mdinfo *info, sighandler_t *sig)
+{
        int rv;
        rv = sysfs_set_num(info, NULL, "suspend_lo", 0x7FFFFFFFFFFFFFFFULL);
        rv |= sysfs_set_num(info, NULL, "suspend_hi", 0);
        rv |= sysfs_set_num(info, NULL, "suspend_lo", 0);
 
-       signal(SIGQUIT, sig[2]);
-       signal(SIGINT, sig[1]);
-       signal(SIGTERM, sig[0]);
+       signal_s(SIGQUIT, sig[2]);
+       signal_s(SIGINT, sig[1]);
+       signal_s(SIGTERM, sig[0]);
 
        if(munlockall() != 0)
                return 3;

diff --git a/util.c b/util.c
index 3d05d074b80ae0495db830c12a5766e502fa2f88..cc94f96ef1205e53f3a51d0bbe1c7eed840e92d7 100644 (file)
--- a/util.c
+++ b/util.c
@@ -35,7 +35,6 @@
 #include       <poll.h>
 #include       <ctype.h>
 #include       <dirent.h>
-#include       <signal.h>
 #include       <dlfcn.h>