Skip to content

CA-406953: fix more undefined behaviour #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
GeraldEV merged 8 commits into from
Mar 5, 2025
Merged

CA-406953: fix more undefined behaviour #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b276416
Update .gitignore with CodeChecker files
edwintorok Feb 21, 2025
9f5f8ec
Tell the compiler that `log_message` takes a format string as argument
edwintorok Feb 21, 2025
2dadb75
CA-406953: fix potential crash in writestatefile
edwintorok Feb 21, 2025
5bcf80f
CA-406953: fix(open): must supply mode with O_CREAT
edwintorok Feb 21, 2025
79ed87b
CA-406953: Use fixed size integer types
edwintorok Feb 21, 2025
809c70e
CA-406953: Fix format string warnings
edwintorok Feb 21, 2025
26a80ad
CA-406953: fix more undefined behaviour
edwintorok Feb 21, 2025
dd04167
Use a single leading 0 for octal permissions
GeraldEV Mar 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@ include/mtcerrno.h
scripts/generr
scripts/generr.o
scripts/ha_errnorc
compile_commands.json
.cache/
3 changes: 2 additions & 1 deletion commands/stubs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@

#include "mtctypes.h"
#include "mtcerrno.h"
#include "log.h"

void
log_message(
Expand All @@ -51,7 +52,7 @@ log_message(
va_list ap;

va_start(ap, fmt);
fprintf(stderr, fmt, ap);
(void)vfprintf(stderr, fmt, ap);
va_end(ap);

fflush(stderr);
Expand Down
11 changes: 6 additions & 5 deletions daemon/com.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include <assert.h>
#include <unistd.h>
#include <signal.h>
#include <inttypes.h>

#include "mtctypes.h"
#include "mtcerrno.h"
Expand Down Expand Up @@ -269,7 +270,7 @@ MTC_STATIC HA_COMMON_OBJECT_HANDLE_INTERNAL
new_handle = malloc(sizeof(HA_COMMON_OBJECT_HANDLE_INTERNAL));
if (new_handle == NULL)
{
log_internal(MTC_LOG_ERR, "COM: cannot allocate for object_handle (size=%d).\n", sizeof(HA_COMMON_OBJECT_HANDLE_INTERNAL));
log_internal(MTC_LOG_ERR, "COM: cannot allocate for object_handle (size=%zu).\n", sizeof(HA_COMMON_OBJECT_HANDLE_INTERNAL));
return NULL;
}
new_handle->object = object;
Expand Down Expand Up @@ -307,7 +308,7 @@ new_object(
new = malloc(sizeof(HA_COMMON_OBJECT));
if (new == NULL)
{
log_internal(MTC_LOG_ERR, "COM: cannot allocate for object (size=%d).\n", sizeof(HA_COMMON_OBJECT));
log_internal(MTC_LOG_ERR, "COM: cannot allocate for object (size=%zu).\n", sizeof(HA_COMMON_OBJECT));
return NULL;
}
new->next = NULL;
Expand Down Expand Up @@ -452,7 +453,7 @@ MTC_STATIC HA_COMMON_OBJECT_CALLBACK_LIST_ITEM
new = malloc(sizeof(HA_COMMON_OBJECT_CALLBACK_LIST_ITEM));
if (new == NULL)
{
log_internal(MTC_LOG_ERR, "COM: cannot allocate for callback (size=%d).\n", sizeof(HA_COMMON_OBJECT_CALLBACK_LIST_ITEM));
log_internal(MTC_LOG_ERR, "COM: cannot allocate for callback (size=%zu).\n", sizeof(HA_COMMON_OBJECT_CALLBACK_LIST_ITEM));
return NULL;
}
new->next = NULL;
Expand Down Expand Up @@ -573,7 +574,7 @@ set_thread_id_record(
return;
}
}
log_message(MTC_LOG_WARNING, "COM: thread_id %d not found in thraed_id_record_table.\n", self);
log_message(MTC_LOG_WARNING, "COM: thread_id %lu not found in thread_id_record_table.\n", self);
break;
}
assert(FALSE);
Expand Down Expand Up @@ -619,7 +620,7 @@ com_log_all_objects(
{
if (object->thread_id_record_table[tid_index].lock_state != LOCK_STATE_NONE)
{
log_message(MTC_LOG_DEBUG, "COM: lock_state=%d thread_id=%x changed_time=%d(ms) .\n",
log_message(MTC_LOG_DEBUG, "COM: lock_state=%d thread_id=0x%lx changed_time=%"PRId64"(ms) .\n",
object->thread_id_record_table[tid_index].lock_state,
object->thread_id_record_table[tid_index].thread_id,
now - object->thread_id_record_table[tid_index].changed_time
Expand Down
5 changes: 3 additions & 2 deletions daemon/heartbeat.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
#include <string.h>
#include <unistd.h>
#include <stdio.h>
#include <inttypes.h>


//
Expand Down Expand Up @@ -1454,7 +1455,7 @@ hb_check_fist(
if (target_delay != 0)
{
log_message(MTC_LOG_DEBUG,
"HB(FIST): heartbeat delay is %d ms\n", target_delay);
"HB(FIST): heartbeat delay is %"PRId64" ms\n", target_delay);

ts = ts_rem = mstots(target_delay);
while (nanosleep(&ts, &ts_rem)) ts = ts_rem;
Expand Down Expand Up @@ -1485,7 +1486,7 @@ hb_check_fist_sticky()
if (target_delay != 0)
{
log_message(MTC_LOG_DEBUG,
"HB(FIST): heartbeat delay is %d ms\n", target_delay);
"HB(FIST): heartbeat delay is %"PRId64" ms\n", target_delay);

ts = mstots(target_delay);
while (nanosleep(&ts, &ts_rem)) ts = ts_rem;
Expand Down
3 changes: 2 additions & 1 deletion daemon/sc_func.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include <unistd.h>
#include <time.h>
#include <assert.h>
#include <inttypes.h>

#include "mtctypes.h"
#include "mtcerrno.h"
Expand Down Expand Up @@ -551,7 +552,7 @@ script_service_do_query_liveset(
if (!xapi_approaching_timeout_reported ||
(logmask & MTC_LOG_MASK_SC_WARNING))
{
log_message(MTC_LOG_WARNING, "SC: (%s) reporting \"Xapi approaching timeout\". now=%d start=%d.\n", __func__, now, xapimon->time_healthcheck_start);
log_message(MTC_LOG_WARNING, "SC: (%s) reporting \"Xapi approaching timeout\". now=%"PRId64" start=%"PRId64".\n", __func__, now, xapimon->time_healthcheck_start);
}
l->xapi_approaching_timeout = TRUE;
}
Expand Down
10 changes: 6 additions & 4 deletions daemon/sm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include <signal.h>
#include <unistd.h>
#include <stdlib.h>
#include <inttypes.h>



Expand Down Expand Up @@ -322,7 +323,8 @@ MTC_STATIC void
print_liveset(
MTC_S32 pri,
PMTC_S8 log_string,
MTC_HOSTMAP hostmap);
MTC_HOSTMAP hostmap)
__attribute__((format(printf, 2, 0)));
Copy link
Collaborator

@freddy77 freddy77 Feb 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? I cannot see the parameters, how can be a formatting string? Maybe it's a bug?

Copy link
Collaborator

@freddy77 freddy77 Feb 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, never mind, the 0 says there are no parameter to check against. Maybe a kind of trick can be in place like

static inline const char *
check_print_liveset_format(const char *fmt, ...) __attribute__((format(printf, 1, 2))
{
    return fmt;
}

#define print_liveset(pri, fmt, map) print_liveset(pri, check_print_liveset_format(fmt, "dummy"), map)

Copy link
Contributor Author

@edwintorok edwintorok Feb 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler is still not entirely happy about it, because it says that the 'log_string' is not a string literal, whereas in the other function where I used format(printf,2,3) it was able to see that fmt is a string literal from the caller and didn't warn about it again.

I'll try the macro trick, that should fix it.


MTC_STATIC void
wait_until_HBSF_state_stable();
Expand Down Expand Up @@ -2289,7 +2291,7 @@ wait_until_HBSF_state_stable()
{
log_maskable_debug_message(FH_TRACE,
"FH: waiting for HB from host (%d),"
" time since last HB receive = %d.\n",
" time since last HB receive = %"PRId64".\n",
index, now - phb->time_last_HB[index]);
logged_hb[index] = TRUE;
}
Expand All @@ -2304,7 +2306,7 @@ wait_until_HBSF_state_stable()
{
log_maskable_debug_message(FH_TRACE,
"FH: waiting for SF from host (%d),"
" time since last SF update = %d.\n",
" time since last SF update = %"PRId64".\n",
index,
now - psf->time_last_SF[index]);
logged_sf[index] = TRUE;
Expand Down Expand Up @@ -2543,7 +2545,7 @@ wait_until_all_hosts_have_consistent_view(
MTC_HOSTMAP_SET(removedhost, selected);
}

log_message(MTC_LOG_WARNING, "after merger:\n", index);
log_message(MTC_LOG_WARNING, "after merger: %d\n", index);
for (index = 0; _is_configured_host(index); index++)
{
MTC_HOSTMAP_INTERSECTION(phb->raw[index].hbdomain, '=',
Expand Down
2 changes: 1 addition & 1 deletion daemon/watchdog.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,7 @@ watchdog_create(
new = malloc(sizeof(WATCHDOG_INSTANCE));
if (new == NULL)
{
log_internal(MTC_LOG_ERR, "WD: cannnot malloc size = %d.\n", sizeof(WATCHDOG_INSTANCE));
log_internal(MTC_LOG_ERR, "WD: cannnot malloc size = %zu.\n", sizeof(WATCHDOG_INSTANCE));
ret = MTC_ERROR_WD_INSUFFICIENT_RESOURCE;
goto error_return;
}
Expand Down
3 changes: 2 additions & 1 deletion daemon/xapi_mon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <inttypes.h>


//
Expand Down Expand Up @@ -438,7 +439,7 @@ xapimon(
if (target_delay != 0)
{
log_message(MTC_LOG_DEBUG,
"XM(FIST): xapi healthcheck delay is %d ms\n", target_delay);
"XM(FIST): xapi healthcheck delay is %"PRId64" ms\n", target_delay);
xm_sleep(target_delay - (_getms() - start));
}

Expand Down
3 changes: 2 additions & 1 deletion include/log.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,8 @@ extern void
log_message(
MTC_S32 priority,
PMTC_S8 fmt,
...);
...)
__attribute__((format (printf, 2, 3)));


// log_bin
Expand Down
9 changes: 5 additions & 4 deletions include/mtctypes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@

#include <sys/time.h>
#include <stddef.h>
#include <stdint.h>

//
//
Expand Down Expand Up @@ -128,11 +129,11 @@ typedef signed short MTC_S16; // 16 bits
// However, int is 4 bytes for both.
//

typedef unsigned int MTC_U32; // 32 bits
typedef signed int MTC_S32; // 32 bits
typedef uint32_t MTC_U32; // 32 bits
typedef int32_t MTC_S32; // 32 bits

typedef unsigned long long MTC_U64; // 64 bits
typedef long long MTC_S64; // 64 bits
typedef uint64_t MTC_U64; // 64 bits
typedef int64_t MTC_S64; // 64 bits


//
Expand Down
2 changes: 1 addition & 1 deletion lib/weightio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@
MTC_STATUS
open_hostweight_file(int *fd, int *err_no)
{
if ((*fd = open(HA_HOST_WEIGHT_FILE, O_RDWR|O_CREAT)) < 0)
if ((*fd = open(HA_HOST_WEIGHT_FILE, O_RDWR|O_CREAT, 0400)) < 0)
{
*err_no = errno;
return MTC_ERROR_WEIGHT_OPEN;
Expand Down
Loading