Skip to content

Commit

Permalink
Merge pull request #22 from edwintorok/private/edvint/warnings
Browse files Browse the repository at this point in the history
CA-407106: fix various XHA warnings from the Clang static analyzer
  • Loading branch information
GeraldEV authored Mar 5, 2025
2 parents 93b821c + 8944196 commit a2582e1
Show file tree
Hide file tree
Showing 10 changed files with 103 additions and 91 deletions.
7 changes: 7 additions & 0 deletions .codechecker.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"analyze": [
"--disable=clang-diagnostic-reserved-macro-identifier",
"--disable=clang-diagnostic-reserved-identifier",
"--disable=clang-diagnostic-unused-parameter"
]
}
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ scripts/generr.o
scripts/ha_errnorc
compile_commands.json
.cache/
*.o
3 changes: 3 additions & 0 deletions commands/cleanupwatchdog.c
Original file line number Diff line number Diff line change
Expand Up @@ -391,13 +391,16 @@ main(
id[idnum++] = strtol(buf, &endptr, 10);
if (*endptr != '\0')
{
(void)fclose(fp);
return MTC_EXIT_INVALID_PARAMETER;
}
if (idnum >= MAX_WATCHDOG_INSTANCE)
{
(void)fclose(fp);
return MTC_EXIT_INVALID_PARAMETER;
}
}
(void)fclose(fp);
for (idindex = 0; idindex < idnum; idindex++)
{
if (id[idindex] == 0)
Expand Down
2 changes: 0 additions & 2 deletions commands/writestatefile.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
53 changes: 15 additions & 38 deletions daemon/log.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
#include <fcntl.h>
#include <sys/stat.h>
#include <dlfcn.h>
#include <stdlib.h>
#include <execinfo.h>
#include <unistd.h>
#include <sys/syscall.h>

Expand Down Expand Up @@ -412,60 +414,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:


log_message(priority, "backtrace -------------\n");
int nptrs = backtrace(return_address, BACKTRACE_SIZE);
if (nptrs <= 0 || (unsigned)nptrs > BACKTRACE_SIZE) {
log_message(priority, "Cannot get backtrace");
return;
}
char **symbols = backtrace_symbols(return_address, BACKTRACE_SIZE);

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;
}
Expand Down
31 changes: 22 additions & 9 deletions daemon/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,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)
Expand Down Expand Up @@ -315,7 +317,7 @@ main(

for (sig = 1; sig < _NSIG; sig++)
{
sigignore(sig);
sigaction(sig, &ignore, NULL);
}

// #### Set resource limit of core size to max value
Expand Down Expand Up @@ -366,7 +368,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++)
Expand Down Expand Up @@ -420,18 +421,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:
Expand Down Expand Up @@ -491,10 +493,21 @@ post_phase_init(
}

// Now it's time to catch signals.
static const 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);
}
}

Expand Down
1 change: 0 additions & 1 deletion daemon/sc_sv.c
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,6 @@ script_service_thread(
memset((void *)&response, 0, sizeof(response));

FD_ZERO(&fds);
nfds = 0;

FD_SET(listening_socket, &fds);
nfds = listening_socket;
Expand Down
93 changes: 54 additions & 39 deletions daemon/sm.c
Original file line number Diff line number Diff line change
Expand Up @@ -811,16 +811,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;
Expand Down Expand Up @@ -901,12 +898,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
Expand Down Expand Up @@ -1867,15 +1874,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;
Expand Down Expand Up @@ -1964,14 +1967,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,
Expand Down Expand Up @@ -2353,13 +2363,10 @@ wait_until_all_hosts_have_consistent_view(
remote_hbdomain_onsf, remote_sfdomain_onhb,
removedhost, tmp_hostmap;

#if 1 // TBD - do we need this timeout?
// TBD - do we need this timeout?
MTC_CLOCK start = _getms();

while (!consistent && _getms() - start < timeout)
#else
while (!consistent)
#endif
do
{
consistent = TRUE;

Expand Down Expand Up @@ -2417,10 +2424,14 @@ wait_until_all_hosts_have_consistent_view(

if (!consistent)
{
if (_getms() - start >= timeout) {
index = -1;
break;
}
mssleep(100);
sm_wait_signals_sm_hb_sf(TRUE, TRUE, TRUE, -1);
}
}
} while(!consistent);

com_reader_lock(sm_object, (void **) &psm);
com_reader_lock(hb_object, (void **) &phb);
Expand Down Expand Up @@ -2781,18 +2792,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)
Expand All @@ -2812,15 +2817,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)
Expand Down
1 change: 0 additions & 1 deletion daemon/xapi_mon.c
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,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();
}
Expand Down
2 changes: 1 addition & 1 deletion include/sm.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit a2582e1

Please sign in to comment.