Skip to content

Commit

Permalink
Refactor: libcrmcommon: New pcmk__mem_assert()
Browse files Browse the repository at this point in the history
Assert without dumping core upon memory allocation failure.

While adding calls to pcmk__mem_assert, update some strdups to
pcmk__str_update() where appropriate.

Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
  • Loading branch information
nrwahl2 committed Mar 13, 2024
1 parent c89ec05 commit 6058956
Show file tree
Hide file tree
Showing 57 changed files with 144 additions and 169 deletions.
2 changes: 1 addition & 1 deletion daemons/attrd/attrd_attributes.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ attrd_create_attribute(xmlNode *xml)
}

a = calloc(1, sizeof(attribute_t));
CRM_ASSERT(a != NULL);
pcmk__mem_assert(a);

if (is_private) {
attrd_set_attr_flags(a, attrd_attr_is_private);
Expand Down
2 changes: 1 addition & 1 deletion daemons/attrd/attrd_cib.c
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ set_alert_attribute_value(GHashTable *t, attribute_value_t *v)
{
attribute_value_t *a_v = NULL;
a_v = calloc(1, sizeof(attribute_value_t));
CRM_ASSERT(a_v != NULL);
pcmk__mem_assert(a_v);

a_v->nodeid = v->nodeid;
a_v->nodename = strdup(v->nodename);
Expand Down
2 changes: 1 addition & 1 deletion daemons/attrd/attrd_corosync.c
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ update_attr_on_host(attribute_t *a, const crm_node_t *peer, const xmlNode *xml,
v = g_hash_table_lookup(a->values, host);
if (v == NULL) {
v = calloc(1, sizeof(attribute_value_t));
CRM_ASSERT(v != NULL);
pcmk__mem_assert(v);

pcmk__str_update(&v->nodename, host);
g_hash_table_replace(a->values, v->nodename, v);
Expand Down
16 changes: 6 additions & 10 deletions daemons/attrd/attrd_sync.c
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,9 @@ attrd_add_client_to_waitlist(pcmk__request_t *request)
}

wl = calloc(sizeof(struct waitlist_node), 1);
pcmk__mem_assert(wl);

CRM_ASSERT(wl != NULL);

wl->client_id = strdup(request->ipc_client->id);

CRM_ASSERT(wl->client_id);
pcmk__str_update(&(wl->client_id), request->ipc_client->id);

if (pcmk__str_eq(sync_point, PCMK__VALUE_LOCAL, pcmk__str_none)) {
wl->sync_point = attrd_sync_point_local;
Expand Down Expand Up @@ -502,22 +499,21 @@ attrd_expect_confirmations(pcmk__request_t *request, attrd_confirmation_action_f
g_hash_table_iter_init(&iter, peer_protocol_vers);
while (g_hash_table_iter_next(&iter, &host, &ver)) {
if (ATTRD_SUPPORTS_CONFIRMATION(GPOINTER_TO_INT(ver))) {
char *s = strdup((char *) host);
char *s = NULL;

CRM_ASSERT(s != NULL);
pcmk__str_update(&s, (char *) host);
respondents = g_list_prepend(respondents, s);
}
}

action = calloc(1, sizeof(struct confirmation_action));
CRM_ASSERT(action != NULL);
pcmk__mem_assert(action);

action->respondents = respondents;
action->fn = fn;
action->xml = pcmk__xml_copy(NULL, request->xml);

action->client_id = strdup(request->ipc_client->id);
CRM_ASSERT(action->client_id != NULL);
pcmk__str_update(&(action->client_id), request->ipc_client->id);

action->ipc_id = request->ipc_id;
action->flags = request->flags;
Expand Down
4 changes: 2 additions & 2 deletions daemons/attrd/attrd_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -288,10 +288,10 @@ attrd_update_minimum_protocol_ver(const char *host, const char *value)
pcmk__scan_min_int(value, &ver, 0);

if (ver > 0) {
char *host_name = strdup(host);
char *host_name = NULL;

/* Record the peer attrd's protocol version. */
CRM_ASSERT(host_name != NULL);
pcmk__str_update(&host_name, host);
g_hash_table_insert(peer_protocol_vers, host_name, GINT_TO_POINTER(ver));

/* If the protocol version is a new minimum, record it as such. */
Expand Down
2 changes: 1 addition & 1 deletion daemons/based/based_callbacks.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ create_cib_reply(const char *op, const char *call_id, const char *client_id,
{
xmlNode *reply = create_xml_node(NULL, PCMK__XE_CIB_REPLY);

CRM_ASSERT(reply != NULL);
pcmk__mem_assert(reply);

crm_xml_add(reply, PCMK__XA_T, PCMK__VALUE_CIB);
crm_xml_add(reply, PCMK__XA_CIB_OP, op);
Expand Down
2 changes: 1 addition & 1 deletion daemons/based/based_messages.c
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ cib_msg_copy(xmlNode *msg)

xmlNode *copy = create_xml_node(NULL, PCMK__XE_COPY);

CRM_ASSERT(copy != NULL);
pcmk__mem_assert(copy);

for (int lpc = 0; lpc < PCMK__NELEM(field_list); lpc++) {
const char *field = field_list[lpc];
Expand Down
2 changes: 1 addition & 1 deletion daemons/based/based_remote.c
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ construct_pam_passwd(int num_msg, const struct pam_message **msg,
CRM_CHECK(num_msg == 1, return PAM_CONV_ERR); /* We only want to handle one message */

reply = calloc(1, sizeof(struct pam_response));
CRM_ASSERT(reply != NULL);
pcmk__mem_assert(reply);

for (count = 0; count < num_msg; ++count) {
switch (msg[count]->msg_style) {
Expand Down
3 changes: 1 addition & 2 deletions daemons/based/based_transaction.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,9 @@ based_transaction_source_str(const pcmk__client_t *client, const char *origin)
pcmk__s(origin, ""));

} else {
source = strdup((origin != NULL)? origin : "unknown source");
pcmk__str_update(&source, pcmk__s(origin, "unknown source"));
}

CRM_ASSERT(source != NULL);
return source;
}

Expand Down
2 changes: 1 addition & 1 deletion daemons/controld/controld_execd.c
Original file line number Diff line number Diff line change
Expand Up @@ -1366,7 +1366,7 @@ new_metadata_cb_data(lrmd_rsc_info_t *rsc, xmlNode *input_xml)
struct metadata_cb_data *data = NULL;

data = calloc(1, sizeof(struct metadata_cb_data));
CRM_ASSERT(data != NULL);
pcmk__mem_assert(data);
data->input_xml = pcmk__xml_copy(NULL, input_xml);
data->rsc = lrmd_copy_rsc_info(rsc);
return data;
Expand Down
2 changes: 1 addition & 1 deletion daemons/controld/controld_messages.c
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ copy_ha_msg_input(ha_msg_input_t * orig)
{
ha_msg_input_t *copy = calloc(1, sizeof(ha_msg_input_t));

CRM_ASSERT(copy != NULL);
pcmk__mem_assert(copy);
copy->msg = (orig != NULL)? pcmk__xml_copy(NULL, orig->msg) : NULL;
copy->xml = get_message_xml(copy->msg, PCMK__XE_CRM_XML);
return copy;
Expand Down
4 changes: 2 additions & 2 deletions daemons/controld/controld_te_events.c
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ update_failcount(const xmlNode *event, const char *event_node_uuid, int rc,
/* Update the fail count, if we're not ignoring failures */
if (!ignore_failures) {
fail_pair = calloc(1, sizeof(pcmk__attrd_query_pair_t));
CRM_ASSERT(fail_pair != NULL);
pcmk__mem_assert(fail_pair);

fail_name = pcmk__failcount_name(rsc_id, task, interval_ms);
fail_pair->name = fail_name;
Expand All @@ -260,7 +260,7 @@ update_failcount(const xmlNode *event, const char *event_node_uuid, int rc,
* so that failure can still be detected and shown, e.g. by crm_mon)
*/
last_pair = calloc(1, sizeof(pcmk__attrd_query_pair_t));
CRM_ASSERT(last_pair != NULL);
pcmk__mem_assert(last_pair);

last_name = pcmk__lastfailure_name(rsc_id, task, interval_ms);
last_pair->name = last_name;
Expand Down
5 changes: 2 additions & 3 deletions daemons/controld/controld_te_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -229,15 +229,14 @@ init_node_pending_timer(const crm_node_t *node, guint timeout)
controld_globals.node_pending_timeout);

node_pending_timer = calloc(1, sizeof(struct abort_timer_s));
CRM_ASSERT(node_pending_timer != NULL);
pcmk__mem_assert(node_pending_timer);

node_pending_timer->aborted = FALSE;
node_pending_timer->priority = PCMK_SCORE_INFINITY;
node_pending_timer->action = pcmk__graph_restart;
node_pending_timer->text = "Node pending timed out";

key = strdup(node->uuid);
CRM_ASSERT(key != NULL);
pcmk__str_update(&key, node->uuid);

g_hash_table_replace(node_pending_timers, key, node_pending_timer);

Expand Down
8 changes: 4 additions & 4 deletions daemons/fenced/fenced_commands.c
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,10 @@ get_action_delay_base(const stonith_device_t *device, const char *action,
hash_value = g_hash_table_lookup(device->params, PCMK_STONITH_DELAY_BASE);

if (hash_value) {
char *value = strdup(hash_value);
char *value = NULL;
char *valptr = value;

CRM_ASSERT(value != NULL);
pcmk__str_update(&value, hash_value);

if (target != NULL) {
for (char *val = strtok(value, "; \t"); val != NULL; val = strtok(NULL, "; \t")) {
Expand Down Expand Up @@ -352,7 +352,7 @@ create_async_command(xmlNode *msg)
}

cmd = calloc(1, sizeof(async_command_t));
CRM_ASSERT(cmd != NULL);
pcmk__mem_assert(cmd);

// All messages must include these
cmd->action = crm_element_value_copy(op, PCMK__XA_ST_DEVICE_ACTION);
Expand Down Expand Up @@ -3262,7 +3262,7 @@ handle_query_request(pcmk__request_t *request)
crm_log_xml_trace(request->xml, "Query");

query = calloc(1, sizeof(struct st_query_data));
CRM_ASSERT(query != NULL);
pcmk__mem_assert(query);

query->reply = fenced_construct_reply(request->xml, NULL, &request->result);
pcmk__str_update(&query->remote_peer, request->peer);
Expand Down
4 changes: 2 additions & 2 deletions daemons/fenced/fenced_remote.c
Original file line number Diff line number Diff line change
Expand Up @@ -1162,7 +1162,7 @@ create_remote_stonith_op(const char *client, xmlNode *request, gboolean peer)
}

op = calloc(1, sizeof(remote_fencing_op_t));
CRM_ASSERT(op != NULL);
pcmk__mem_assert(op);

crm_element_value_int(request, PCMK__XA_ST_TIMEOUT, &(op->base_timeout));
// Value -1 means disable any static/random fencing delays
Expand Down Expand Up @@ -2169,7 +2169,7 @@ add_device_properties(const xmlNode *xml, remote_fencing_op_t *op,
int flags = st_device_supports_on; /* Old nodes that don't set the flag assume they support the on action */

/* Add a new entry to this peer's devices list */
CRM_ASSERT(props != NULL);
pcmk__mem_assert(props);
g_hash_table_insert(peer->devices, strdup(device), props);

/* Peers with verified (monitored) access will be preferred */
Expand Down
2 changes: 1 addition & 1 deletion daemons/schedulerd/schedulerd_messages.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ init_working_set(void)
{
pcmk_scheduler_t *scheduler = pe_new_working_set();

CRM_ASSERT(scheduler != NULL);
pcmk__mem_assert(scheduler);

crm_config_error = FALSE;
crm_config_warning = FALSE;
Expand Down
18 changes: 17 additions & 1 deletion include/crm/common/logging_internal.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2015-2023 the Pacemaker project contributors
* Copyright 2015-2024 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
Expand Down Expand Up @@ -61,6 +61,22 @@ void pcmk__set_config_warning_handler(pcmk__config_warning_func warning_handler,
} \
} while (0)

/*!
* \internal
* \brief Abort without dumping core if a pointer is \c NULL
*
* This is intended to check for memory allocation failure, rather than for null
* pointers in general.
*
* \param[in] ptr Pointer to check
*/
#define pcmk__mem_assert(ptr) do { \
if ((ptr) == NULL) { \
crm_abort(__FILE__, __func__, __LINE__, "Out of memory", FALSE, \
FALSE); \
} \
} while (0)

/*!
* \internal
* \brief Execute code depending on whether trace logging is enabled
Expand Down
3 changes: 0 additions & 3 deletions lib/cib/cib_file.c
Original file line number Diff line number Diff line change
Expand Up @@ -933,9 +933,6 @@ cib_file_write_with_digest(xmlNode *cib_root, const char *cib_dirname,
char *tmp_cib = crm_strdup_printf("%s/cib.XXXXXX", cib_dirname);
char *tmp_digest = crm_strdup_printf("%s/cib.XXXXXX", cib_dirname);

CRM_ASSERT((cib_path != NULL) && (digest_path != NULL)
&& (tmp_cib != NULL) && (tmp_digest != NULL));

/* Ensure the admin didn't modify the existing CIB underneath us */
crm_trace("Reading cluster configuration file %s", cib_path);
rc = cib_file_read_and_verify(cib_path, NULL, NULL);
Expand Down
3 changes: 1 addition & 2 deletions lib/cib/cib_ops.c
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,7 @@ update_counter(xmlNode *xml_obj, const char *field, bool reset)
int_value = atoi(old_value);
new_value = pcmk__itoa(++int_value);
} else {
new_value = strdup("1");
CRM_ASSERT(new_value != NULL);
pcmk__str_update(&new_value, "1");
}

crm_trace("Update %s from %s to %s",
Expand Down
2 changes: 1 addition & 1 deletion lib/cluster/cluster.c
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ pcmk_cluster_new(void)
{
crm_cluster_t *cluster = calloc(1, sizeof(crm_cluster_t));

CRM_ASSERT(cluster != NULL);
pcmk__mem_assert(cluster);
return cluster;
}

Expand Down
4 changes: 2 additions & 2 deletions lib/cluster/cpg.c
Original file line number Diff line number Diff line change
Expand Up @@ -698,8 +698,8 @@ pcmk_cpg_membership(cpg_handle_t handle,
uint32_t local_nodeid = get_local_nodeid(handle);
const struct cpg_address **sorted;

sorted = malloc(member_list_entries * sizeof(const struct cpg_address *));
CRM_ASSERT(sorted != NULL);
sorted = calloc(member_list_entries, sizeof(const struct cpg_address *));
pcmk__mem_assert(sorted);

for (size_t iter = 0; iter < member_list_entries; iter++) {
sorted[iter] = member_list + iter;
Expand Down
12 changes: 4 additions & 8 deletions lib/cluster/membership.c
Original file line number Diff line number Diff line change
Expand Up @@ -603,8 +603,7 @@ pcmk__purge_node_from_cache(const char *node_name, uint32_t node_id)
/* node_name could be a pointer into the cache entry being purged,
* so reassign it to a copy before the original gets freed
*/
node_name_copy = strdup(node_name);
CRM_ASSERT(node_name_copy != NULL);
pcmk__str_update(&node_name_copy, node_name);
node_name = node_name_copy;

crm_trace("Purging %s from Pacemaker Remote node cache", node_name);
Expand Down Expand Up @@ -1317,13 +1316,10 @@ known_node_cache_refresh_helper(xmlNode *xml_node, void *user_data)
char *uniqueid = crm_generate_uuid();

node = calloc(1, sizeof(crm_node_t));
CRM_ASSERT(node != NULL);

node->uname = strdup(uname);
CRM_ASSERT(node->uname != NULL);
pcmk__mem_assert(node);

node->uuid = strdup(id);
CRM_ASSERT(node->uuid != NULL);
pcmk__str_update(&(node->uname), uname);
pcmk__str_update(&(node->uuid), id);

g_hash_table_replace(known_node_cache, uniqueid, node);

Expand Down
2 changes: 1 addition & 1 deletion lib/common/acl.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ create_acl(const xmlNode *xml, GList *acls, enum xml_private_flags mode)
}

acl = calloc(1, sizeof (xml_acl_t));
CRM_ASSERT(acl != NULL);
pcmk__mem_assert(acl);

acl->mode = mode;
if (xpath) {
Expand Down
4 changes: 2 additions & 2 deletions lib/common/actions.c
Original file line number Diff line number Diff line change
Expand Up @@ -320,12 +320,12 @@ parse_op_key(const char *key, char **rsc_id, char **op_type, guint *interval_ms)
// Set output variables
if (rsc_id != NULL) {
*rsc_id = strndup(key, action_underbar);
CRM_ASSERT(*rsc_id != NULL);
pcmk__mem_assert(*rsc_id);
}
if (op_type != NULL) {
*op_type = strndup(key + action_underbar + 1,
interval_underbar - action_underbar - 1);
CRM_ASSERT(*op_type != NULL);
pcmk__mem_assert(*op_type);
}
if (interval_ms != NULL) {
*interval_ms = local_interval_ms;
Expand Down
5 changes: 2 additions & 3 deletions lib/common/io.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2004-2022 the Pacemaker project contributors
* Copyright 2004-2024 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
Expand Down Expand Up @@ -638,8 +638,7 @@ pcmk__full_path(const char *filename, const char *dirname)
CRM_ASSERT(filename != NULL);

if (filename[0] == '/') {
path = strdup(filename);
CRM_ASSERT(path != NULL);
pcmk__str_update(&path, filename);

} else {
CRM_ASSERT(dirname != NULL);
Expand Down
2 changes: 1 addition & 1 deletion lib/common/ipc_attrd.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ set_pairs_data(pcmk__attrd_api_reply_t *data, xmlNode *msg_data)
node != NULL; node = crm_next_same_xml(node)) {
pair = calloc(1, sizeof(pcmk__attrd_query_pair_t));

CRM_ASSERT(pair != NULL);
pcmk__mem_assert(pair);

pair->node = crm_element_value(node, PCMK__XA_ATTR_HOST);
pair->name = name;
Expand Down
2 changes: 1 addition & 1 deletion lib/common/ipc_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ pcmk__new_ipc_event(void)
{
struct iovec *iov = calloc(2, sizeof(struct iovec));

CRM_ASSERT(iov != NULL);
pcmk__mem_assert(iov);
return iov;
}

Expand Down
Loading

0 comments on commit 6058956

Please sign in to comment.