File bsc#1133866-0001-Refactor-libcrmcommon-improve-parse_op_key-efficienc-1.1.patch of Package pacemaker.13547
From 76fd8c326f38a7427b2d878959bc5da268940fea Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Wed, 30 May 2018 17:49:25 -0500
Subject: [PATCH] Refactor: libcrmcommon: improve parse_op_key() efficiency
- Check error conditions more strictly
- Allow output variables to be NULL if caller if not interested in them
- Don't over-allocate memory
- Ensure output variables need to be freed only if return value is TRUE
---
crmd/te_events.c | 2 --
lib/common/utils.c | 48 ++++++++++++++++++++++++++++++++----------------
pengine/allocate.c | 11 ++---------
pengine/constraints.c | 13 +------------
tools/crm_mon.c | 3 +--
5 files changed, 36 insertions(+), 41 deletions(-)
Index: pacemaker-1.1.16+20170320.77ea74d/crmd/te_events.c
===================================================================
--- pacemaker-1.1.16+20170320.77ea74d.orig/crmd/te_events.c
+++ pacemaker-1.1.16+20170320.77ea74d/crmd/te_events.c
@@ -147,8 +147,6 @@ update_failcount(xmlNode * event, const
CRM_CHECK(on_uname != NULL, return TRUE);
CRM_CHECK(parse_op_key(id, &rsc_id, &task, &interval),
crm_err("Couldn't parse: %s", ID(event)); goto bail);
- CRM_CHECK(task != NULL, goto bail);
- CRM_CHECK(rsc_id != NULL, goto bail);
/* Decide whether update is necessary and what value to use */
if ((interval > 0) || safe_str_eq(task, CRMD_ACTION_PROMOTE)
Index: pacemaker-1.1.16+20170320.77ea74d/lib/common/utils.c
===================================================================
--- pacemaker-1.1.16+20170320.77ea74d.orig/lib/common/utils.c
+++ pacemaker-1.1.16+20170320.77ea74d/lib/common/utils.c
@@ -655,10 +655,22 @@ parse_op_key(const char *key, char **rsc
char *mutable_key = NULL;
char *mutable_key_ptr = NULL;
int len = 0, offset = 0, ch = 0;
+ int local_interval_ms = 0;
- CRM_CHECK(key != NULL, return FALSE);
+ // Initialize output variables in case of early return
+ if (rsc_id) {
+ *rsc_id = NULL;
+ }
+ if (op_type) {
+ *op_type = NULL;
+ }
+ if (interval) {
+ *interval = 0;
+ }
- *interval = 0;
+ CRM_CHECK(key && *key, return FALSE);
+
+ // Parse interval at end of string
len = strlen(key);
offset = len - 1;
@@ -674,36 +686,36 @@ parse_op_key(const char *key, char **rsc
digits--;
ch = ch * 10;
}
- *interval += ch;
+ local_interval_ms += ch;
offset--;
}
+ crm_trace("Operation key '%s' has interval %ums", key, local_interval_ms);
+ if (interval) {
+ *interval = local_interval_ms;
+ }
+
+ CRM_CHECK((offset != (len - 1)) && (key[offset] == '_'), return FALSE);
- crm_trace(" Interval: %d", *interval);
- CRM_CHECK(key[offset] == '_', return FALSE);
-
- mutable_key = strdup(key);
- mutable_key[offset] = 0;
+ mutable_key = strndup(key, offset);
offset--;
while (offset > 0 && key[offset] != '_') {
offset--;
}
- CRM_CHECK(key[offset] == '_', free(mutable_key);
- return FALSE);
+ CRM_CHECK(key[offset] == '_',
+ free(mutable_key); return FALSE);
mutable_key_ptr = mutable_key + offset + 1;
crm_trace(" Action: %s", mutable_key_ptr);
-
- *op_type = strdup(mutable_key_ptr);
+ if (op_type) {
+ *op_type = strdup(mutable_key_ptr);
+ }
mutable_key[offset] = 0;
offset--;
- CRM_CHECK(mutable_key != mutable_key_ptr, free(mutable_key);
- return FALSE);
-
notify = strstr(mutable_key, "_post_notify");
if (notify && safe_str_eq(notify, "_post_notify")) {
notify[0] = 0;
@@ -715,7 +727,11 @@ parse_op_key(const char *key, char **rsc
}
crm_trace(" Resource: %s", mutable_key);
- *rsc_id = mutable_key;
+ if (rsc_id) {
+ *rsc_id = mutable_key;
+ } else {
+ free(mutable_key);
+ }
return TRUE;
}
Index: pacemaker-1.1.16+20170320.77ea74d/pengine/allocate.c
===================================================================
--- pacemaker-1.1.16+20170320.77ea74d.orig/pengine/allocate.c
+++ pacemaker-1.1.16+20170320.77ea74d/pengine/allocate.c
@@ -1663,15 +1663,11 @@ find_actions_by_task(GListPtr actions, r
if (list == NULL) {
/* we're potentially searching a child of the original resource */
char *key = NULL;
- char *tmp = NULL;
char *task = NULL;
int interval = 0;
- if (parse_op_key(original_key, &tmp, &task, &interval)) {
+ if (parse_op_key(original_key, NULL, &task, &interval)) {
key = generate_op_key(rsc->id, task, interval);
- /* crm_err("looking up %s instead of %s", key, original_key); */
- /* slist_iter(action, action_t, actions, lpc, */
- /* crm_err(" - %s", action->uuid)); */
list = find_actions(actions, key, NULL);
} else {
@@ -1679,7 +1675,6 @@ find_actions_by_task(GListPtr actions, r
}
free(key);
- free(tmp);
free(task);
}
@@ -1760,11 +1755,10 @@ rsc_order_first(resource_t * lh_rsc, ord
if (lh_actions == NULL && lh_rsc != rh_rsc) {
char *key = NULL;
- char *rsc_id = NULL;
char *op_type = NULL;
int interval = 0;
- parse_op_key(order->lh_action_task, &rsc_id, &op_type, &interval);
+ parse_op_key(order->lh_action_task, NULL, &op_type, &interval);
key = generate_op_key(lh_rsc->id, op_type, interval);
if (lh_rsc->fns->state(lh_rsc, TRUE) == RSC_ROLE_STOPPED && safe_str_eq(op_type, RSC_STOP)) {
@@ -1785,7 +1779,6 @@ rsc_order_first(resource_t * lh_rsc, ord
}
free(op_type);
- free(rsc_id);
}
gIter = lh_actions;
Index: pacemaker-1.1.16+20170320.77ea74d/pengine/constraints.c
===================================================================
--- pacemaker-1.1.16+20170320.77ea74d.orig/pengine/constraints.c
+++ pacemaker-1.1.16+20170320.77ea74d/pengine/constraints.c
@@ -1348,23 +1348,12 @@ static char *
task_from_action_or_key(action_t *action, const char *key)
{
char *res = NULL;
- char *rsc_id = NULL;
- char *op_type = NULL;
- int interval = 0;
if (action) {
res = strdup(action->task);
} else if (key) {
- int rc = 0;
- rc = parse_op_key(key, &rsc_id, &op_type, &interval);
- if (rc == TRUE) {
- res = op_type;
- op_type = NULL;
- }
- free(rsc_id);
- free(op_type);
+ parse_op_key(key, NULL, &res, NULL);
}
-
return res;
}
Index: pacemaker-1.1.16+20170320.77ea74d/tools/crm_mon.c
===================================================================
--- pacemaker-1.1.16+20170320.77ea74d.orig/tools/crm_mon.c
+++ pacemaker-1.1.16+20170320.77ea74d/tools/crm_mon.c
@@ -3790,7 +3790,6 @@ handle_rsc_op(xmlNode * xml, const char
int rc = -1;
int status = -1;
int action = -1;
- int interval = 0;
int target_rc = -1;
int transition_num = -1;
gboolean notify = TRUE;
@@ -3834,7 +3833,7 @@ handle_rsc_op(xmlNode * xml, const char
return;
}
- if (parse_op_key(id, &rsc, &task, &interval) == FALSE) {
+ if (parse_op_key(id, &rsc, &task, NULL) == FALSE) {
crm_err("Invalid event detected for %s", id);
goto bail;
}
