File bsc#1133866-0001-Refactor-libcrmcommon-improve-... of Package pacemaker.16898

Overview Repositories Revisions Requests Users Attributes Meta

File bsc#1133866-0001-Refactor-libcrmcommon-improve-parse_op_key-efficienc.patch of Package pacemaker.16898

From be8bf91d9b7fac635e2d671c9d34eb13a1b7699d 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 --
 pengine/allocate.c    | 11 ++-------
 pengine/constraints.c | 13 +---------
 lib/common/operations.c                | 45 ++++++++++++++++++++++------------
 tools/crm_mon.c                        |  3 +--
 5 files changed, 34 insertions(+), 40 deletions(-)

Index: pacemaker-1.1.18+20180430.b12c320f5/crmd/te_events.c
===================================================================
--- pacemaker-1.1.18+20180430.b12c320f5.orig/crmd/te_events.c
+++ pacemaker-1.1.18+20180430.b12c320f5/crmd/te_events.c
@@ -136,8 +136,6 @@ update_failcount(xmlNode * event, const
     CRM_CHECK(on_uname != NULL, return TRUE);
     CRM_CHECK(parse_op_key(id, &rsc_id, &task, &interval_ms),
               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_ms > 0) || safe_str_eq(task, CRMD_ACTION_PROMOTE)
Index: pacemaker-1.1.18+20180430.b12c320f5/pengine/allocate.c
===================================================================
--- pacemaker-1.1.18+20180430.b12c320f5.orig/pengine/allocate.c
+++ pacemaker-1.1.18+20180430.b12c320f5/pengine/allocate.c
@@ -1639,15 +1639,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;
         guint interval_ms = 0;
 
-        if (parse_op_key(original_key, &tmp, &task, &interval_ms)) {
+        if (parse_op_key(original_key, NULL, &task, &interval_ms)) {
             key = generate_op_key(rsc->id, task, interval_ms);
-            /* 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 {
@@ -1655,7 +1651,6 @@ find_actions_by_task(GListPtr actions, r
         }
 
         free(key);
-        free(tmp);
         free(task);
     }
 
@@ -1737,11 +1732,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;
         guint interval_ms = 0;
 
-        parse_op_key(order->lh_action_task, &rsc_id, &op_type, &interval_ms);
+        parse_op_key(order->lh_action_task, NULL, &op_type, &interval_ms);
         key = generate_op_key(lh_rsc->id, op_type, interval_ms);
 
         if (lh_rsc->fns->state(lh_rsc, TRUE) == RSC_ROLE_STOPPED && safe_str_eq(op_type, RSC_STOP)) {
@@ -1762,7 +1756,6 @@ rsc_order_first(resource_t * lh_rsc, ord
         }
 
         free(op_type);
-        free(rsc_id);
     }
 
     gIter = lh_actions;
Index: pacemaker-1.1.18+20180430.b12c320f5/pengine/constraints.c
===================================================================
--- pacemaker-1.1.18+20180430.b12c320f5.orig/pengine/constraints.c
+++ pacemaker-1.1.18+20180430.b12c320f5/pengine/constraints.c
@@ -1378,23 +1378,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;
-    guint interval_ms = 0;
 
     if (action) {
         res = strdup(action->task);
     } else if (key) {
-        int rc = 0;
-        rc = parse_op_key(key, &rsc_id, &op_type, &interval_ms);
-        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.18+20180430.b12c320f5/lib/common/operations.c
===================================================================
--- pacemaker-1.1.18+20180430.b12c320f5.orig/lib/common/operations.c
+++ pacemaker-1.1.18+20180430.b12c320f5/lib/common/operations.c
@@ -49,11 +49,22 @@ parse_op_key(const char *key, char **rsc
     char *mutable_key_ptr = NULL;
     size_t len = 0, offset = 0;
     unsigned long long ch = 0;
+    guint 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_ms) {
+        *interval_ms = 0;
+    }
+
+    CRM_CHECK(key && *key, return FALSE);
 
     // Parse interval at end of string
-    *interval_ms = 0;
     len = strlen(key);
     offset = len - 1;
     while ((offset > 0) && isdigit(key[offset])) {
@@ -61,36 +72,36 @@ parse_op_key(const char *key, char **rsc
         for (int digits = len - offset; digits > 1; --digits) {
             ch = ch * 10;
         }
-        *interval_ms += ch;
+        local_interval_ms += ch;
         offset--;
     }
-    crm_trace("Operation key '%s' has interval %ums", key, *interval_ms);
+    crm_trace("Operation key '%s' has interval %ums", key, local_interval_ms);
+    if (interval_ms) {
+        *interval_ms = local_interval_ms;
+    }
 
-    CRM_CHECK(key[offset] == '_', return FALSE);
+    CRM_CHECK((offset != (len - 1)) && (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;
@@ -102,7 +113,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.18+20180430.b12c320f5/tools/crm_mon.c
===================================================================
--- pacemaker-1.1.18+20180430.b12c320f5.orig/tools/crm_mon.c
+++ pacemaker-1.1.18+20180430.b12c320f5/tools/crm_mon.c
@@ -3312,7 +3312,6 @@ handle_rsc_op(xmlNode * xml, const char
     int rc = -1;
     int status = -1;
     int action = -1;
-    guint interval_ms = 0;
     int target_rc = -1;
     int transition_num = -1;
     gboolean notify = TRUE;
@@ -3356,7 +3355,7 @@ handle_rsc_op(xmlNode * xml, const char
         return;
     }
 
-    if (parse_op_key(id, &rsc, &task, &interval_ms) == FALSE) {
+    if (parse_op_key(id, &rsc, &task, NULL) == FALSE) {
         crm_err("Invalid event detected for %s", id);
         goto bail;
     }

Places

File bsc#1133866-0001-Refactor-libcrmcommon-improve-parse_op_key-efficienc.patch of Package pacemaker.16898

Places