Skip to content

Commit 1e37e5d

Browse files
committed
Refactor: libcrmcomon: New pcmk__xe_get_int()
To replace crm_element_value_int(). Note that, like the other recently added getter functions, this leaves dest unchanged on error. crm_element_value_int() set dest to -1. This should indirectly improve some of the callers, which were not error-checking crm_element_value_int()'s return code and simply assumed the value was valid and not -1. And IMO, it's better not to log errors or warnings within the getter function as crm_element_value_int() was doing. The caller has more context and can log errors or warnings if appropriate. Sometimes, a parse failure may be expected as a legitimate possibility. The one case I found questionable was the getting of PCMK__XA_ST_DELAY in create_remote_stonith_op(). If that value is set to -1, then all delays are disabled. crm_element_value_int() sets the value to -1 upon a parse error, and this function was not doing any error checking before now. So I wondered if we were relying on the parse error defaulting to -1. However, I don't think it's a problem. It looks like that attribute is only ever getting set by crm_xml_add_int() -- so even if it's set based on user input, it's set to a valid integer. The -1 value seems to be intended only for when it's set explicitly to -1. We simply haven't error-checked the parsing before now. See commit a0cbd52 and associated commits in PR ClusterLabs#2027. Signed-off-by: Reid Wahl <[email protected]>
1 parent 83cc1a5 commit 1e37e5d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+202
-166
lines changed

daemons/attrd/attrd_attributes.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2013-2024 the Pacemaker project contributors
2+
* Copyright 2013-2025 the Pacemaker project contributors
33
*
44
* The version control history for this file may have further details.
55
*
@@ -38,7 +38,7 @@ attrd_create_attribute(xmlNode *xml)
3838
/* Set type is meaningful only when writing to the CIB. Private
3939
* attributes are not written.
4040
*/
41-
crm_element_value_int(xml, PCMK__XA_ATTR_IS_PRIVATE, &is_private);
41+
pcmk__xe_get_int(xml, PCMK__XA_ATTR_IS_PRIVATE, &is_private);
4242
if (!is_private && !pcmk__str_any_of(set_type,
4343
PCMK_XE_INSTANCE_ATTRIBUTES,
4444
PCMK_XE_UTILIZATION, NULL)) {

daemons/attrd/attrd_corosync.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2013-2024 the Pacemaker project contributors
2+
* Copyright 2013-2025 the Pacemaker project contributors
33
*
44
* The version control history for this file may have further details.
55
*
@@ -81,7 +81,7 @@ attrd_peer_message(pcmk__node_status_t *peer, xmlNode *xml)
8181
* response so the originating peer knows what they're a confirmation
8282
* for.
8383
*/
84-
crm_element_value_int(xml, PCMK__XA_CALL_ID, &callid);
84+
pcmk__xe_get_int(xml, PCMK__XA_CALL_ID, &callid);
8585
reply = attrd_confirmation(callid);
8686

8787
/* And then send the confirmation back to the originating peer. This
@@ -243,7 +243,7 @@ update_attr_on_host(attribute_t *a, const pcmk__node_status_t *peer,
243243
}
244244

245245
// If value is for a Pacemaker Remote node, remember that
246-
crm_element_value_int(xml, PCMK__XA_ATTR_IS_REMOTE, &is_remote);
246+
pcmk__xe_get_int(xml, PCMK__XA_ATTR_IS_REMOTE, &is_remote);
247247
if (is_remote) {
248248
attrd_set_value_flags(v, attrd_value_remote);
249249
pcmk__assert(pcmk__cluster_lookup_remote_node(host) != NULL);
@@ -299,8 +299,7 @@ update_attr_on_host(attribute_t *a, const pcmk__node_status_t *peer,
299299
} else {
300300
int is_force_write = 0;
301301

302-
crm_element_value_int(xml, PCMK__XA_ATTRD_IS_FORCE_WRITE,
303-
&is_force_write);
302+
pcmk__xe_get_int(xml, PCMK__XA_ATTRD_IS_FORCE_WRITE, &is_force_write);
304303

305304
if (is_force_write == 1 && a->timeout_ms && a->timer) {
306305
/* Save forced writing and set change flag. */

daemons/attrd/attrd_elections.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2013-2024 the Pacemaker project contributors
2+
* Copyright 2013-2025 the Pacemaker project contributors
33
*
44
* The version control history for this file may have further details.
55
*
@@ -108,7 +108,7 @@ attrd_check_for_new_writer(const pcmk__node_status_t *peer, const xmlNode *xml)
108108
{
109109
int peer_state = 0;
110110

111-
crm_element_value_int(xml, PCMK__XA_ATTR_WRITER, &peer_state);
111+
pcmk__xe_get_int(xml, PCMK__XA_ATTR_WRITER, &peer_state);
112112
if (peer_state == election_won) {
113113
if ((election_state(attrd_cluster) == election_won)
114114
&& !pcmk__str_eq(peer->name, attrd_cluster->priv->node_name,

daemons/attrd/attrd_ipc.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ attrd_client_peer_remove(pcmk__request_t *request)
151151
if (host == NULL) {
152152
int nodeid = 0;
153153

154-
crm_element_value_int(xml, PCMK__XA_ATTR_HOST_ID, &nodeid);
154+
pcmk__xe_get_int(xml, PCMK__XA_ATTR_HOST_ID, &nodeid);
155155
if (nodeid > 0) {
156156
pcmk__node_status_t *node = NULL;
157157
char *host_alloc = NULL;

daemons/attrd/attrd_messages.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,8 @@ handle_confirm_request(pcmk__request_t *request)
107107

108108
crm_debug("Received confirmation from %s", request->peer);
109109

110-
if (crm_element_value_int(request->xml, PCMK__XA_CALL_ID,
111-
&callid) == -1) {
110+
if (pcmk__xe_get_int(request->xml, PCMK__XA_CALL_ID,
111+
&callid) != pcmk_rc_ok) {
112112
pcmk__set_result(&request->result, CRM_EX_PROTOCOL, PCMK_EXEC_INVALID,
113113
"Could not get callid from XML");
114114
} else {

daemons/attrd/attrd_sync.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2022-2024 the Pacemaker project contributors
2+
* Copyright 2022-2025 the Pacemaker project contributors
33
*
44
* The version control history for this file may have further details.
55
*
@@ -240,7 +240,7 @@ attrd_ack_waitlist_clients(enum attrd_sync_point sync_point, const xmlNode *xml)
240240
return;
241241
}
242242

243-
if (crm_element_value_int(xml, PCMK__XA_CALL_ID, &callid) == -1) {
243+
if (pcmk__xe_get_int(xml, PCMK__XA_CALL_ID, &callid) != pcmk_rc_ok) {
244244
crm_warn("Could not get callid from request XML");
245245
return;
246246
}
@@ -484,7 +484,8 @@ attrd_expect_confirmations(pcmk__request_t *request, attrd_confirmation_action_f
484484
expected_confirmations = pcmk__intkey_table((GDestroyNotify) free_action);
485485
}
486486

487-
if (crm_element_value_int(request->xml, PCMK__XA_CALL_ID, &callid) == -1) {
487+
if (pcmk__xe_get_int(request->xml, PCMK__XA_CALL_ID,
488+
&callid) != pcmk_rc_ok) {
488489
crm_err("Could not get callid from xml");
489490
return;
490491
}

daemons/based/based_callbacks.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ do_local_notify(const xmlNode *notify_src, const char *client_id,
171171

172172
CRM_CHECK((notify_src != NULL) && (client_id != NULL), return);
173173

174-
crm_element_value_int(notify_src, PCMK__XA_CIB_CALLID, &msg_id);
174+
pcmk__xe_get_int(notify_src, PCMK__XA_CIB_CALLID, &msg_id);
175175

176176
client_obj = pcmk__find_client_by_id(client_id);
177177
if (client_obj == NULL) {
@@ -259,8 +259,7 @@ cib_common_callback_worker(uint32_t id, uint32_t flags, xmlNode * op_request,
259259
const char *type = crm_element_value(op_request,
260260
PCMK__XA_CIB_NOTIFY_TYPE);
261261

262-
crm_element_value_int(op_request, PCMK__XA_CIB_NOTIFY_ACTIVATE,
263-
&on_off);
262+
pcmk__xe_get_int(op_request, PCMK__XA_CIB_NOTIFY_ACTIVATE, &on_off);
264263

265264
crm_debug("Setting %s callbacks %s for client %s",
266265
type, (on_off? "on" : "off"), pcmk__client_name(cib_client));

daemons/controld/controld_execd_state.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2024 the Pacemaker project contributors
2+
* Copyright 2012-2025 the Pacemaker project contributors
33
*
44
* The version control history for this file may have further details.
55
*
@@ -571,7 +571,7 @@ crmd_remote_proxy_cb(lrmd_t *lrmd, void *userdata, xmlNode *msg)
571571
CRM_OP_NODE_INFO, pcmk__str_none)) {
572572
int node_id = 0;
573573

574-
crm_element_value_int(request, PCMK_XA_ID, &node_id);
574+
pcmk__xe_get_int(request, PCMK_XA_ID, &node_id);
575575
if ((node_id <= 0)
576576
&& (crm_element_value(request, PCMK_XA_UNAME) == NULL)) {
577577
crm_xml_add(request, PCMK_XA_UNAME, lrm_state->node_name);
@@ -592,7 +592,7 @@ crmd_remote_proxy_cb(lrmd_t *lrmd, void *userdata, xmlNode *msg)
592592
crm_xml_add(op_reply, PCMK_XA_FUNCTION, __func__);
593593
crm_xml_add_int(op_reply, PCMK__XA_LINE, __LINE__);
594594

595-
crm_element_value_int(msg, PCMK__XA_LRMD_IPC_MSG_ID, &msg_id);
595+
pcmk__xe_get_int(msg, PCMK__XA_LRMD_IPC_MSG_ID, &msg_id);
596596
remote_proxy_relay_response(proxy, op_reply, msg_id);
597597

598598
pcmk__xml_free(op_reply);

daemons/controld/controld_join_client.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2004-2024 the Pacemaker project contributors
2+
* Copyright 2004-2025 the Pacemaker project contributors
33
*
44
* The version control history for this file may have further details.
55
*
@@ -278,7 +278,7 @@ do_cl_join_finalize_respond(long long action,
278278
was_nack = FALSE;
279279
}
280280

281-
crm_element_value_int(input->msg, PCMK__XA_JOIN_ID, &join_id);
281+
pcmk__xe_get_int(input->msg, PCMK__XA_JOIN_ID, &join_id);
282282

283283
if (was_nack) {
284284
crm_err("Shutting down because cluster join with leader %s failed "

daemons/controld/controld_join_dc.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2004-2024 the Pacemaker project contributors
2+
* Copyright 2004-2025 the Pacemaker project contributors
33
*
44
* The version control history for this file may have further details.
55
*
@@ -464,7 +464,7 @@ do_dc_join_filter_offer(long long action,
464464
join_node = pcmk__get_node(0, join_from, NULL,
465465
pcmk__node_search_cluster_member);
466466

467-
crm_element_value_int(join_ack->msg, PCMK__XA_JOIN_ID, &join_id);
467+
pcmk__xe_get_int(join_ack->msg, PCMK__XA_JOIN_ID, &join_id);
468468
if (join_id != current_join_id) {
469469
crm_debug("Ignoring join-%d request from %s because we are on join-%d",
470470
join_id, join_from, current_join_id);
@@ -795,7 +795,8 @@ do_dc_join_ack(long long action,
795795
goto done;
796796
}
797797

798-
if (crm_element_value_int(join_ack->msg, PCMK__XA_JOIN_ID, &join_id) != 0) {
798+
if (pcmk__xe_get_int(join_ack->msg, PCMK__XA_JOIN_ID,
799+
&join_id) != pcmk_rc_ok) {
799800
crm_warn("Ignoring join confirmation from %s without valid join ID",
800801
join_from);
801802
goto done;

0 commit comments

Comments
 (0)