From: Svetlana Shmidt <sshmidt@google.com>
Date: Thu, 10 Sep 2020 10:58:11 +0000 (+0000)
Subject: simplify distribution_cmp function
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0aa1c4fafcd55301126ede161410d316cc4e64f8;p=thirdparty%2Fcollectd.git

simplify distribution_cmp function
---

diff --git a/src/daemon/distribution.c b/src/daemon/distribution.c
index 9d85b3bf9..0f3907f9e 100644
--- a/src/daemon/distribution.c
+++ b/src/daemon/distribution.c
@@ -341,65 +341,53 @@ double distribution_squared_deviation_sum(distribution_t *dist) {
   return squared_deviation_sum;
 }
 
-static int compare_longint(uint64_t a, uint64_t b) {
+static int compare_uint64(uint64_t a, uint64_t b) {
   return a == b ? 0 : a < b ? -1 : 1;
 }
 
-/** return a pointer to array int[2].
- * The first value is error code or 0 if succeed.
- * The second value is result of comparison
- */
-static int *distribution_cmp(distribution_t *d1, distribution_t *d2) {
-  int *comparison = calloc(2, sizeof(*comparison));
-  if (comparison == NULL) {
-    return NULL;
-  }
+/* distribution_cmp compares d1 and d2 and stores the result of the comparison in result:
+ *     1 if d1 is larger than d2,
+ *    -1 if d1 is smaller than d2, and
+ *     0 if d1 and d2 are equal.
+ * On error an error code is returned and result is undefined.
+ * If distributions don't exist or don't have the same structure EINVAL will be returned
+ * If distributions have* the same structure but are uncomparable ERANGE will be returned */
+static int distribution_cmp(distribution_t *d1, distribution_t *d2, int *result) {
   if (d1 == NULL || d2 == NULL) {
-    comparison[0] = EINVAL;
-    return comparison;
+    return EINVAL;
   }
   if (d1->num_buckets != d2->num_buckets) {
-    comparison[0] = EINVAL;
-    return comparison;
+    return EINVAL;
   }
   for (size_t i = 0; i < tree_size(d1->num_buckets); i++) {
     if (d1->tree[i].maximum !=
         d2->tree[i].maximum) { // there can be a trouble with double comparison
                                // but we assume that distributions were created
                                // in the same way
-      comparison[0] = EINVAL;
-      return comparison;
+      return EINVAL;
     }
   }
 
-  int res =
-      compare_longint(d1->tree[0].bucket_counter, d2->tree[0].bucket_counter);
+  *result =
+      compare_uint64(d1->tree[0].bucket_counter, d2->tree[0].bucket_counter);
   for (size_t i = 1; i < tree_size(d1->num_buckets); i++) {
     int cur_res =
-        compare_longint(d1->tree[i].bucket_counter, d2->tree[i].bucket_counter);
-    if (res != cur_res) {
-      if (cur_res == 0)
-        continue;
-      if (res == 0) {
-        res = cur_res;
-        continue;
-      }
-      comparison[0] = ERANGE;
-      break;
+        compare_uint64(d1->tree[i].bucket_counter, d2->tree[i].bucket_counter);
+    if (cur_res != 0 && cur_res != *result)  {
+      return ERANGE;
     }
   }
-  comparison[1] = res;
-  return comparison;
+  return 0;
 }
 
 bool distribution_equal(distribution_t *d1, distribution_t *d2) {
   pthread_mutex_lock(&d1->mutex);
   pthread_mutex_lock(&d2->mutex);
-  int *cmp = distribution_cmp(d1, d2);
-  bool ans = (cmp[0] == 0 && cmp[1] == 0);
+  int cmp_result;
+  int cmp_status = distribution_cmp(d1, d2, &cmp_result);
+  bool ans = (cmp_status == 0 && cmp_result == 0);
   pthread_mutex_unlock(&d2->mutex);
   pthread_mutex_unlock(&d1->mutex);
-  free(cmp);
   return ans;
 }
 
@@ -408,16 +396,14 @@ int distribution_sub(distribution_t *d1, distribution_t *d2) {
   pthread_mutex_lock(&d1->mutex);
   pthread_mutex_lock(&d2->mutex);
 
-  int *cmp_status = distribution_cmp(d1, d2);
-  if (cmp_status[0] != 0 ||
-      cmp_status[1] == -1) { // i.e. d1 < d2 or can't compare
-    int status = cmp_status[0];
-    if (cmp_status[1] == -1)
-      status = ERANGE;
+  int cmp_result = 0;
+  int cmp_status = distribution_cmp(d1, d2, &cmp_status);
+  if (cmp_status != 0 || cmp_result == -1) { // i.e. d1 < d2 or can't compare
+    if (cmp_result == -1)
+      cmp_status = ERANGE;
     pthread_mutex_unlock(&d2->mutex);
     pthread_mutex_unlock(&d1->mutex);
-    free(cmp_status);
-    return status;
+    return cmp_status;
   }
 
   d1->total_sum -= d2->total_sum;
@@ -428,6 +414,5 @@ int distribution_sub(distribution_t *d1, distribution_t *d2) {
 
   pthread_mutex_unlock(&d2->mutex);
   pthread_mutex_unlock(&d1->mutex);
-  free(cmp_status);
   return 0;
 }