Refactor: libcrmcommon: Return (gchar *) from pcmk__digest_xml()

To avoid a pointless string copy

Signed-off-by: Reid Wahl <[email protected]>

Author: nrwahl2

daemons/based/based_callbacks.c

@@ -367,7 +367,7 @@ cib_common_callback(qb_ipcs_connection_t * c, void *data, size_t size, gboolean
 }
 
 static uint64_t ping_seq = 0;
-static char *ping_digest = NULL;
+static gchar *ping_digest = NULL;
 static bool ping_modified_since = FALSE;
 
 static gboolean
@@ -378,7 +378,7 @@ cib_digester_cb(gpointer data)
         xmlNode *ping = pcmk__xe_create(NULL, PCMK__XE_PING);
 
         ping_seq++;
-        free(ping_digest);
+        g_free(ping_digest);
         ping_digest = NULL;
         ping_modified_since = FALSE;
         snprintf(buffer, 32, "%" PRIu64, ping_seq);

daemons/based/based_messages.c

@@ -140,7 +140,7 @@ cib_process_ping(const char *op, int options, const char *section, xmlNode * req
 {
     const char *host = pcmk__xe_get(req, PCMK__XA_SRC);
     const char *seq = pcmk__xe_get(req, PCMK__XA_CIB_PING_ID);
-    char *digest = pcmk__digest_xml(the_cib, true);
+    gchar *digest = pcmk__digest_xml(the_cib, true);
 
     xmlNode *wrapper = NULL;
 
@@ -177,7 +177,7 @@ cib_process_ping(const char *op, int options, const char *section, xmlNode * req
              pcmk__xe_get(existing_cib, PCMK_XA_EPOCH),
              pcmk__xe_get(existing_cib, PCMK_XA_NUM_UPDATES));
 
-    free(digest);
+    g_free(digest);
 
     return pcmk_ok;
 }
@@ -406,7 +406,7 @@ int
 sync_our_cib(xmlNode * request, gboolean all)
 {
     int result = pcmk_ok;
-    char *digest = NULL;
+    gchar *digest = NULL;
     const char *host = pcmk__xe_get(request, PCMK__XA_SRC);
     const char *op = pcmk__xe_get(request, PCMK__XA_CIB_OP);
     pcmk__node_status_t *peer = NULL;
@@ -448,7 +448,7 @@ sync_our_cib(xmlNode * request, gboolean all)
         result = -ENOTCONN;
     }
     pcmk__xml_free(replace_request);
-    free(digest);
+    g_free(digest);
     return result;
 }
 

daemons/schedulerd/schedulerd_messages.c

@@ -53,13 +53,13 @@ handle_pecalc_request(pcmk__request_t *request)
     xmlNode *wrapper = pcmk__xe_first_child(msg, PCMK__XE_CRM_XML, NULL, NULL);
     xmlNode *xml_data = pcmk__xe_first_child(wrapper, NULL, NULL, NULL);
 
-    static char *last_digest = NULL;
+    static gchar *last_digest = NULL;
     static char *filename = NULL;
 
     unsigned int seq = 0U;
     int series_id = 0;
     int series_wrap = 0;
-    char *digest = NULL;
+    gchar *digest = NULL;
     const char *value = NULL;
     time_t execution_date = time(NULL);
     xmlNode *converted = NULL;
@@ -79,14 +79,14 @@ handle_pecalc_request(pcmk__request_t *request)
         pcmk__xe_set_int(scheduler->priv->graph, "transition_id", 0);
         pcmk__xe_set_int(scheduler->priv->graph, PCMK_OPT_CLUSTER_DELAY, 0);
         process = false;
-        free(digest);
+        g_free(digest);
 
     } else if (pcmk__str_eq(digest, last_digest, pcmk__str_casei)) {
         is_repoke = true;
-        free(digest);
+        g_free(digest);
 
     } else {
-        free(last_digest);
+        g_free(last_digest);
         last_digest = digest;
     }
 

include/crm/common/digest_internal.h

@@ -43,7 +43,7 @@ typedef struct {
 
 gchar *pcmk__digest_on_disk_cib(const xmlNode *input);
 gchar *pcmk__digest_operation(const xmlNode *input);
-char *pcmk__digest_xml(const xmlNode *input, bool filter);
+gchar *pcmk__digest_xml(const xmlNode *input, bool filter);
 
 bool pcmk__verify_digest(const xmlNode *input, const char *expected);
 

lib/cib/cib_ops.c

@@ -343,7 +343,7 @@ cib_process_replace(const char *op, int options, const char *section, xmlNode *
         const char *digest = pcmk__xe_get(req, PCMK__XA_DIGEST);
 
         if (digest) {
-            char *digest_verify = pcmk__digest_xml(input, true);
+            gchar *digest_verify = pcmk__digest_xml(input, true);
 
             if (!pcmk__str_eq(digest_verify, digest, pcmk__str_casei)) {
                 crm_err("Digest mis-match on replace from %s: %s vs. %s (expected)", peer,
@@ -353,7 +353,7 @@ cib_process_replace(const char *op, int options, const char *section, xmlNode *
             } else {
                 crm_info("Digest matched on replace from %s: %s", peer, digest);
             }
-            free(digest_verify);
+            g_free(digest_verify);
 
         } else {
             crm_trace("No digest to verify");

lib/common/digest.c

@@ -194,25 +194,25 @@ pcmk__digest_operation(const xmlNode *input)
  * \param[in] filter  Whether to filter certain XML attributes
  *
  * \return Newly allocated string containing digest
+ *
+ * \note The caller is responsible for freeing the return value using
+ *       \c g_free().
  */
-char *
+gchar *
 pcmk__digest_xml(const xmlNode *xml, bool filter)
 {
     /* @TODO Filtering accounts for significant CPU usage. Consider removing if
      * possible.
      */
     GString *buf = g_string_sized_new(1024);
-    gchar *digest_g = NULL;
-    char *digest = NULL;
+    gchar *digest = NULL;
 
     pcmk__xml_string(xml, (filter? pcmk__xml_fmt_filtered : 0), buf, 0);
-    digest_g = pcmk__md5sum(buf->str);
-    if (digest_g == NULL) {
+    digest = pcmk__md5sum(buf->str);
+    if (digest == NULL) {
         goto done;
     }
 
-    digest = pcmk__str_copy(digest_g);
-
     pcmk__if_tracing(
         {
             char *trace_file = pcmk__assert_asprintf("digest-%s", digest);
@@ -230,7 +230,6 @@ pcmk__digest_xml(const xmlNode *xml, bool filter)
 
 done:
     g_string_free(buf, TRUE);
-    g_free(digest_g);
     return digest;
 }
 
@@ -386,10 +385,11 @@ char *
 calculate_xml_versioned_digest(xmlNode *input, gboolean sort,
                                gboolean do_filter, const char *version)
 {
+    gchar *digest_g = NULL;
+    char *digest = NULL;
+
     if ((version == NULL) || (compare_version("3.0.5", version) > 0)) {
         xmlNode *sorted = NULL;
-        gchar *digest_g = NULL;
-        char *digest = NULL;
 
         if (sort) {
             xmlNode *sorted = sorted_xml(input, NULL, true);
@@ -401,14 +401,16 @@ calculate_xml_versioned_digest(xmlNode *input, gboolean sort,
                   pcmk__s(version, "unknown feature set"));
 
         digest_g = calculate_xml_digest_v1(input);
-        digest = pcmk__str_copy(digest_g);
-
         pcmk__xml_free(sorted);
-        g_free(digest_g);
-        return digest;
+
+    } else {
+        crm_trace("Using v2 digest algorithm for %s", version);
+        digest_g = pcmk__digest_xml(input, do_filter);
     }
-    crm_trace("Using v2 digest algorithm for %s", version);
-    return pcmk__digest_xml(input, do_filter);
+
+    digest = pcmk__str_copy(digest_g);
+    g_free(digest_g);
+    return digest;
 }
 
 char *

lib/common/patchset.c

@@ -297,7 +297,7 @@ xml_create_patchset(int format, xmlNode *source, xmlNode *target,
 void
 pcmk__xml_patchset_add_digest(xmlNode *patchset, const xmlNode *target)
 {
-    char *digest = NULL;
+    gchar *digest = NULL;
 
     CRM_CHECK((patchset != NULL) && (target != NULL), return);
 
@@ -310,7 +310,7 @@ pcmk__xml_patchset_add_digest(xmlNode *patchset, const xmlNode *target)
     digest = pcmk__digest_xml(target, true);
 
     pcmk__xe_set(patchset, PCMK__XA_DIGEST, digest);
-    free(digest);
+    g_free(digest);
 }
 
 /*!
@@ -843,9 +843,8 @@ xml_apply_patchset(xmlNode *xml, const xmlNode *patchset, bool check_version)
     }
 
     if ((rc == pcmk_ok) && (digest != NULL)) {
-        char *new_digest = NULL;
+        gchar *new_digest = pcmk__digest_xml(xml, true);
 
-        new_digest = pcmk__digest_xml(xml, true);
         if (!pcmk__str_eq(new_digest, digest, pcmk__str_casei)) {
             crm_info("v%d digest mis-match: expected %s, calculated %s",
                      format, digest, new_digest);
@@ -864,7 +863,7 @@ xml_apply_patchset(xmlNode *xml, const xmlNode *patchset, bool check_version)
             crm_trace("v%d digest matched: expected %s, calculated %s",
                       format, digest, new_digest);
         }
-        free(new_digest);
+        g_free(new_digest);
     }
     pcmk__xml_free(old);
     return rc;
@@ -980,7 +979,7 @@ void
 patchset_process_digest(xmlNode *patch, const xmlNode *source,
                         const xmlNode *target, bool with_digest)
 {
-    char *digest = NULL;
+    gchar *digest = NULL;
 
     if ((patch == NULL) || (source == NULL) || (target == NULL)
         || !with_digest) {
@@ -996,9 +995,7 @@ patchset_process_digest(xmlNode *patch, const xmlNode *source,
     digest = pcmk__digest_xml(target, true);
 
     pcmk__xe_set(patch, PCMK__XA_DIGEST, digest);
-    free(digest);
-
-    return;
+    g_free(digest);
 }
 
 // LCOV_EXCL_STOP

tools/cibadmin.c

@@ -738,7 +738,7 @@ main(int argc, char **argv)
         goto done;
 
     } else if (strcmp(options.cib_action, "md5-sum-versioned") == 0) {
-        char *digest = NULL;
+        gchar *digest = NULL;
 
         if (input == NULL) {
             exit_code = CRM_EX_USAGE;
@@ -749,7 +749,7 @@ main(int argc, char **argv)
 
         digest = pcmk__digest_xml(input, true);
         fprintf(stdout, "%s\n", pcmk__s(digest, "<null>"));
-        free(digest);
+        g_free(digest);
         goto done;
 
     } else if (pcmk__str_eq(options.cib_action, PCMK__CIB_REQUEST_MODIFY,