CA-407106: Fix various build and static analyzer warnings

Also disable warnings we can't/don't want to fix now: * clang-diagnostic-reserved-macro-identifier The bug is in Xen, it mandates using `__XEN_TOOLS__` in `sysctl.h` Signed-off-by: Edwin Török <[email protected]>
xenserver · Feb 24, 2025 · d7919a1 · d7919a1
7 parents 0a303fb + 6b06570 + f24ab91 + 48c0c55 + d55135a + 738ac06 + 3759f67
commit d7919a1
Show file tree

Hide file tree

Showing 8 changed files with 87 additions and 86 deletions.
diff --git a/commands/cleanupwatchdog.c b/commands/cleanupwatchdog.c
@@ -398,6 +398,7 @@ main(
             return MTC_EXIT_INVALID_PARAMETER;
         }
     }
+    (void)fclose(fp);
     for (idindex = 0; idindex < idnum; idindex++)
     {
         if (id[idindex] == 0) 

diff --git a/commands/writestatefile.c b/commands/writestatefile.c
@@ -156,8 +156,6 @@ main(
         exit(status_to_exit(MTC_ERROR_SF_OPEN));
     }
 
-    status = MTC_ERROR_INVALID_PARAMETER;
-
     if (strcmp(argv[1], "setinit") == 0)
     {
         status = global_init_state(sf);

diff --git a/daemon/log.c b/daemon/log.c
@@ -49,6 +49,8 @@
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <dlfcn.h>
+#include <stdlib.h>
+#include <execinfo.h>
 #include <unistd.h>
 
 
@@ -447,60 +449,35 @@ log_logmask(void)
 //
 //
 
-#define BACKTRACE_SIZE 10
-#define FILL_RETURN_ADDRESS(x)                          \
-    if (__builtin_frame_address(x) == 0)                \
-    {                                                   \
-        return_address[x] = NULL;                       \
-        goto end_fill;                                  \
-    }                                                   \
-    return_address[x] = __builtin_return_address(x);    \
-
-
+#define BACKTRACE_SIZE 10        
 
 void
 log_backtrace(MTC_S32 priority)
 {
     int level;
-    Dl_info dli;
-    void *return_address[BACKTRACE_SIZE] = {NULL};
-
-    level = 0;
-
-    FILL_RETURN_ADDRESS(0);
-    FILL_RETURN_ADDRESS(1);
-    FILL_RETURN_ADDRESS(2);
-    FILL_RETURN_ADDRESS(3);
-    FILL_RETURN_ADDRESS(4);
-    FILL_RETURN_ADDRESS(5);
-    FILL_RETURN_ADDRESS(6);
-    FILL_RETURN_ADDRESS(7);
-    FILL_RETURN_ADDRESS(8);
-    FILL_RETURN_ADDRESS(9);
- end_fill:
-
+    void* return_address[BACKTRACE_SIZE] = {NULL};
+
     log_message(priority, "backtrace -------------\n");
+    int nptrs = backtrace(return_address, sizeof(return_address) / sizeof(return_address[0]));
+    if (nptrs <= 0 || (unsigned)nptrs > sizeof(return_address) / sizeof(return_address[0])) {
+        log_message(priority, "Cannot get backtrace");
+        return;
+    }
+    char** symbols = backtrace_symbols(return_address, sizeof(return_address) / sizeof(return_address[0]));
 
     for (level = 0;
-         level < BACKTRACE_SIZE;
+         level < nptrs;
          level ++)
     {
-        int err;
-
-        if (return_address[level] == NULL)
-        {
-            break;
+        if (symbols && symbols[level]) {
+            log_message(priority, "  %2d: (%p) %s\n", level, return_address[level], symbols[level]);
         }
-        err = dladdr(return_address[level], &dli);
-        if (err == 0 || dli.dli_sname == NULL) 
+        else
         {
             log_message(priority, "  %2d: (%p) --\n", level, return_address[level]);
         }
-        else 
-        {
-            log_message(priority, "  %2d: (%p) %s\n", level, return_address[level], dli.dli_sname);
-        }
     }
+    free(symbols);
     log_message(priority, "backtrace -------------\n");
     return;
 }

diff --git a/daemon/main.c b/daemon/main.c
@@ -220,7 +220,9 @@ main(
     //  logrotate may send SIGHUP regardless of the state of xha. 
     //
 
-    sigignore(SIGHUP);
+    struct sigaction ignore = { 0 };
+    ignore.sa_handler = SIG_IGN;
+    sigaction(SIGHUP, &ignore, NULL);
 
     //  Initialize ha_config in BSS.
     //  (static initialization causes a warning for unknown reason)
@@ -336,7 +338,7 @@ main(
 
     for (sig = 1; sig < _NSIG; sig++)
     {
-        sigignore(sig);
+        sigaction(sig, &ignore, NULL);
     }
 
     //  #### Set resource limit of core size to max value
@@ -387,7 +389,6 @@ main(
 
     //  #### Initialize internal modules (phase 0 and 1)
 
-    init_index = 0;
     for (phase = 0; phase < 2; phase++)
     {
         for (init_index = 0; init_index < N_INIT_FUNCS; init_index++)
@@ -441,18 +442,19 @@ main(
 //
 //  sigcatch -
 //
-//  signal handler for sigset().
+//  signal handler for sigaction().
 //
 //  sigcatch may be invoked in any thread context in any time.
 //  Acquiring any lock which does not support recursive in this function causes deadlock.
 //
+//  action.sa_flags is 0, which means that the signal that triggerred the handler
+//  is automatically blocked already
+//
 
 MTC_STATIC void
 sigcatch(
     int signo)
 {
-    sigset(signo, sigcatch);
-
     switch (signo)
     {
     case SIGTERM:
@@ -512,10 +514,21 @@ post_phase_init(
         }
 
         //  Now it's time to catch signals.
+        int signals[] = { SIGTERM, SIGCHLD, SIGHUP };
+        sigset_t mask;
+        unsigned i;
+        struct sigaction action = { 0 };
+        action.sa_handler = sigcatch;
+        sigemptyset(&mask);
+
+        for (i = 0; i < sizeof(signals) / sizeof(signals[0]); i++) {
+            int sig = signals[i];
+            sigaddset(&mask, sig);
+
+            sigaction(sig, &action, NULL);
+        }
 
-        sigset(SIGTERM, sigcatch);
-        sigset(SIGCHLD, sigcatch);
-        sigset(SIGHUP, sigcatch);
+        sigprocmask(SIG_UNBLOCK, &mask, NULL);
     }
 }
 

diff --git a/daemon/sc_sv.c b/daemon/sc_sv.c
@@ -321,7 +321,6 @@ script_service_thread(
         memset((void *)&response, 0, sizeof(response));
 
         FD_ZERO(&fds);
-        nfds = 0;
 
         FD_SET(listening_socket, &fds);
         nfds = listening_socket;

diff --git a/daemon/sm.c b/daemon/sm.c
@@ -808,16 +808,13 @@ sm(
 }
 
 
-MTC_STATIC void
-rendezvous(
-    SM_PHASE    phase1,
-    SM_PHASE    phase2,
-    SM_PHASE    phase3,
-    MTC_BOOLEAN on_heartbeat,
-    MTC_BOOLEAN on_statefile)
-{
 #if RENDEZVOUS_FAULT_HANDLING
-    void rendezvous_wait(SM_PHASE p1, SM_PHASE p2)
+MTC_STATIC void
+rendezvous_wait(
+     SM_PHASE p1,
+     SM_PHASE p2,
+     MTC_BOOLEAN on_heartbeat,
+     MTC_BOOLEAN on_statefile)
     {
         PCOM_DATA_SM    psm;
         PCOM_DATA_HB    phb;
@@ -898,12 +895,22 @@ rendezvous(
             hb_SF_cancel_accelerate();
         }
     }
+#endif
 
+MTC_STATIC void
+rendezvous(
+    SM_PHASE    phase1,
+    SM_PHASE    phase2,
+    SM_PHASE    phase3,
+    MTC_BOOLEAN on_heartbeat,
+    MTC_BOOLEAN on_statefile)
+{
+#if RENDEZVOUS_FAULT_HANDLING
     set_sm_phase(phase1);
-    rendezvous_wait(phase1, phase2);
+    rendezvous_wait(phase1, phase2, on_heartbeat, on_statefile);
 
     set_sm_phase(phase2);
-    rendezvous_wait(phase2, phase3);
+    rendezvous_wait(phase2, phase3, on_heartbeat, on_statefile);
 #else
     set_sm_phase(phase2);
 #endif
@@ -1863,15 +1870,11 @@ check_pool_state()
 
 
 MTC_STATIC MTC_BOOLEAN
-update_sfdomain()
-{
-    MTC_CLOCK       now;
-    MTC_S8          hostmap[MAX_HOST_NUM + 1] = {0};
-    MTC_BOOLEAN     changed = FALSE;
-
-    MTC_BOOLEAN update_sfdomain_sub(
+update_sfdomain_sub(
+        MTC_CLOCK now,
+        MTC_S8 hostmap[MAX_HOST_NUM + 1],
         MTC_BOOLEAN writable)
-    {
+{
         PCOM_DATA_SF    psf;
         MTC_BOOLEAN changed = FALSE;
         MTC_S32     index;
@@ -1960,14 +1963,21 @@ update_sfdomain()
         }
 
         return changed;
-    }
+}
+
+MTC_STATIC MTC_BOOLEAN
+update_sfdomain()
+{
+    MTC_CLOCK       now;
+    MTC_S8          hostmap[MAX_HOST_NUM + 1] = {0};
+    MTC_BOOLEAN     changed = FALSE;
 
     now = _getms();
 
-    changed = update_sfdomain_sub(FALSE);
+    changed = update_sfdomain_sub(now, hostmap, FALSE);
     if (changed)
     {
-        changed = update_sfdomain_sub(TRUE);
+        changed = update_sfdomain_sub(now, hostmap, TRUE);
         if (changed)
         {
             log_message(MTC_LOG_DEBUG,
@@ -2778,18 +2788,12 @@ sm_send_signals_sm_hb_sf(
 }
 
 
-MTC_STATIC MTC_BOOLEAN
-sm_wait_signals_sm_hb_sf(
+static MTC_BOOLEAN
+check_sigs(
     MTC_BOOLEAN sm_sig,
     MTC_BOOLEAN hb_sig,
-    MTC_BOOLEAN sf_sig,
-    MTC_CLOCK   timeout)
+    MTC_BOOLEAN sf_sig)
 {
-    MTC_BOOLEAN signaled;
-    MTC_CLOCK   start = _getms();
-
-    MTC_BOOLEAN check_sigs()
-    {
         MTC_BOOLEAN signaled = FALSE;
 
         if (sm_sig && smvar.sm_sig)
@@ -2809,15 +2813,25 @@ sm_wait_signals_sm_hb_sf(
         }
 
         return signaled;
-    }
+}
+
+MTC_STATIC MTC_BOOLEAN
+sm_wait_signals_sm_hb_sf(
+    MTC_BOOLEAN sm_sig,
+    MTC_BOOLEAN hb_sig,
+    MTC_BOOLEAN sf_sig,
+    MTC_CLOCK   timeout)
+{
+    MTC_BOOLEAN signaled;
+    MTC_CLOCK   start = _getms();
 
     if (timeout == 0)
     {
         return FALSE;
     }
 
     pthread_mutex_lock(&smvar.mutex);
-    while (!(signaled = check_sigs()) &&
+    while (!(signaled = check_sigs(sm_sig, hb_sig, sf_sig)) &&
            ((timeout < 0)? TRUE: (_getms() - start < timeout)))
     {
         if (timeout < 0)

diff --git a/daemon/xapi_mon.c b/daemon/xapi_mon.c
@@ -866,7 +866,6 @@ wait_pid_timeout(
     while ((ret_pid = waitpid(pid, pstat, WNOHANG)) == 0 &&
            now < start_time + timeout)
     {
-        ret_pid = 0;
         nanosleep(&sleep_ts, &sleep_rem);
         now = _getms();
     }

diff --git a/include/sm.h b/include/sm.h
@@ -365,7 +365,7 @@ sm_initialize(
     MTC_S32 phase);
 
 extern void
-self_fence();
+self_fence(MTC_STATUS code, PMTC_S8 message);
 
 extern MTC_BOOLEAN
 sm_get_join_block();