From 8efa00d966c890bf3a193c6eb5bf72ade1b9d14d Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Tue, 27 Mar 2018 15:05:47 -0500
Subject: [PATCH] Refactor: lrmd: functionize creating reply

reduces duplication, and enforces consistency in reply XML and logging
---
 lrmd/lrmd.c | 113 ++++++++++++++++++++--------------------------------
 1 file changed, 43 insertions(+), 70 deletions(-)

diff --git a/lrmd/lrmd.c b/lrmd/lrmd.c
index 4a423b897..874f25d15 100644
--- a/lrmd/lrmd.c
+++ b/lrmd/lrmd.c
@@ -362,24 +350,15 @@ schedule_lrmd_cmd(lrmd_rsc_t * rsc, lrmd_cmd_t * cmd)
     }
 }
 
-static void
-send_reply(crm_client_t * client, int rc, uint32_t id, int call_id)
+static xmlNode *
+create_lrmd_reply(const char *origin, int rc, int call_id)
 {
-    int send_rc = 0;
-    xmlNode *reply = NULL;
+    xmlNode *reply = create_xml_node(NULL, T_LRMD_REPLY);
 
-    reply = create_xml_node(NULL, T_LRMD_REPLY);
-    crm_xml_add(reply, F_LRMD_ORIGIN, __FUNCTION__);
+    crm_xml_add(reply, F_LRMD_ORIGIN, origin);
     crm_xml_add_int(reply, F_LRMD_RC, rc);
     crm_xml_add_int(reply, F_LRMD_CALLID, call_id);
-
-    send_rc = lrmd_server_send_reply(client, id, reply);
-
-    free_xml(reply);
-    if (send_rc < 0) {
-        crm_warn("Reply to client %s failed: %s " CRM_XS " %d",
-                 client->name, pcmk_strerror(send_rc), send_rc);
-    }
+    return reply;
 }
 
 static void
@@ -1283,23 +1262,24 @@ free_rsc(gpointer data)
     free(rsc);
 }
 
-static int
-process_lrmd_signon(crm_client_t * client, uint32_t id, xmlNode * request)
+static xmlNode *
+process_lrmd_signon(crm_client_t *client, xmlNode *request, int call_id)
 {
-    xmlNode *reply = create_xml_node(NULL, "reply");
+    xmlNode *reply = NULL;
+    int rc = pcmk_ok;
     const char *is_ipc_provider = crm_element_value(request, F_LRMD_IS_IPC_PROVIDER);
     const char *protocol_version = crm_element_value(request, F_LRMD_PROTOCOL_VERSION);
 
     if (compare_version(protocol_version, LRMD_MIN_PROTOCOL_VERSION) < 0) {
         crm_err("Cluster API version must be greater than or equal to %s, not %s",
                 LRMD_MIN_PROTOCOL_VERSION, protocol_version);
-        crm_xml_add_int(reply, F_LRMD_RC, -EPROTO);
+        rc = -EPROTO;
     }
 
+    reply = create_lrmd_reply(__FUNCTION__, rc, call_id);
     crm_xml_add(reply, F_LRMD_OPERATION, CRM_OP_REGISTER);
     crm_xml_add(reply, F_LRMD_CLIENTID, client->id);
     crm_xml_add(reply, F_LRMD_PROTOCOL_VERSION, LRMD_PROTOCOL_VERSION);
-    lrmd_server_send_reply(client, id, reply);
 
     if (crm_is_true(is_ipc_provider)) {
         /* this is a remote connection from a cluster nodes crmd */
@@ -1307,9 +1287,7 @@ process_lrmd_signon(crm_client_t * client, uint32_t id, xmlNode * request)
         ipc_proxy_add_provider(client);
 #endif
     }
-
-    free_xml(reply);
-    return pcmk_ok;
+    return reply;
 }
 
 static int
@@ -1337,52 +1315,34 @@ process_lrmd_rsc_register(crm_client_t * client, uint32_t id, xmlNode * request)
     return rc;
 }
 
-static void
-process_lrmd_get_rsc_info(crm_client_t * client, uint32_t id, xmlNode * request)
+static xmlNode *
+process_lrmd_get_rsc_info(xmlNode *request, int call_id)
 {
     int rc = pcmk_ok;
-    int send_rc = 0;
-    int call_id = 0;
     xmlNode *rsc_xml = get_xpath_object("//" F_LRMD_RSC, request, LOG_ERR);
     const char *rsc_id = crm_element_value(rsc_xml, F_LRMD_RSC_ID);
     xmlNode *reply = NULL;
     lrmd_rsc_t *rsc = NULL;
 
-    crm_element_value_int(request, F_LRMD_CALLID, &call_id);
-
-    if (!rsc_id) {
+    if (rsc_id == NULL) {
         rc = -ENODEV;
-        goto get_rsc_done;
-    }
-
-    if (!(rsc = g_hash_table_lookup(rsc_list, rsc_id))) {
-        crm_info("Resource '%s' not found (%d active resources)",
-                 rsc_id, g_hash_table_size(rsc_list));
-        rc = -ENODEV;
-        goto get_rsc_done;
+    } else {
+        rsc = g_hash_table_lookup(rsc_list, rsc_id);
+        if (rsc == NULL) {
+            crm_info("Resource '%s' not found (%d active resources)",
+                     rsc_id, g_hash_table_size(rsc_list));
+            rc = -ENODEV;
+        }
     }
 
-  get_rsc_done:
-
-    reply = create_xml_node(NULL, T_LRMD_REPLY);
-    crm_xml_add(reply, F_LRMD_ORIGIN, __FUNCTION__);
-    crm_xml_add_int(reply, F_LRMD_RC, rc);
-    crm_xml_add_int(reply, F_LRMD_CALLID, call_id);
-
+    reply = create_lrmd_reply(__FUNCTION__, rc, call_id);
     if (rsc) {
         crm_xml_add(reply, F_LRMD_RSC_ID, rsc->rsc_id);
         crm_xml_add(reply, F_LRMD_CLASS, rsc->class);
         crm_xml_add(reply, F_LRMD_PROVIDER, rsc->provider);
         crm_xml_add(reply, F_LRMD_TYPE, rsc->type);
     }
-
-    send_rc = lrmd_server_send_reply(client, id, reply);
-
-    if (send_rc < 0) {
-        crm_warn("LRMD reply to %s failed: %d", client->name, send_rc);
-    }
-
-    free_xml(reply);
+    return reply;
 }
 
 static int
@@ -1568,6 +1528,7 @@ process_lrmd_message(crm_client_t * client, uint32_t id, xmlNode * request)
     const char *op = crm_element_value(request, F_LRMD_OPERATION);
     int do_reply = 0;
     int do_notify = 0;
+    xmlNode *reply = NULL;
 
     crm_trace("Processing %s operation from %s", op, client->id);
     crm_element_value_int(request, F_LRMD_CALLID, &call_id);
@@ -1578,13 +1539,15 @@ process_lrmd_message(crm_client_t * client, uint32_t id, xmlNode * request)
 #endif
         do_reply = 1;
     } else if (crm_str_eq(op, CRM_OP_REGISTER, TRUE)) {
-        rc = process_lrmd_signon(client, id, request);
+        reply = process_lrmd_signon(client, request, call_id);
+        do_reply = 1;
     } else if (crm_str_eq(op, LRMD_OP_RSC_REG, TRUE)) {
         rc = process_lrmd_rsc_register(client, id, request);
         do_notify = 1;
         do_reply = 1;
     } else if (crm_str_eq(op, LRMD_OP_RSC_INFO, TRUE)) {
-        process_lrmd_get_rsc_info(client, id, request);
+        reply = process_lrmd_get_rsc_info(request, call_id);
+        do_reply = 1;
     } else if (crm_str_eq(op, LRMD_OP_RSC_UNREG, TRUE)) {
         rc = process_lrmd_rsc_unregister(client, id, request);
         /* don't notify anyone about failed un-registers */
@@ -1620,7 +1583,17 @@ process_lrmd_message(crm_client_t * client, uint32_t id, xmlNode * request)
               op, client->id, rc, do_reply, do_notify);
 
     if (do_reply) {
-        send_reply(client, rc, id, call_id);
+        int send_rc = pcmk_ok;
+
+        if (reply == NULL) {
+            reply = create_lrmd_reply(__FUNCTION__, rc, call_id);
+        }
+        send_rc = lrmd_server_send_reply(client, id, reply);
+        free_xml(reply);
+        if (send_rc < 0) {
+            crm_warn("Reply to client %s failed: %s " CRM_XS " %d",
+                     client->name, pcmk_strerror(send_rc), send_rc);
+        }
     }
 
     if (do_notify) {
-- 
2.26.2

