GRPC metadata review cleanup

diff --git a/src/daemon/utils_cache.c b/src/daemon/utils_cache.c
index 3534c3d..ea7c3e3 100644 (file)
--- a/src/daemon/utils_cache.c
+++ b/src/daemon/utils_cache.c
@@ -917,11 +917,6 @@ int uc_iterator_get_meta(uc_iter_t *iter, meta_data_t **ret_meta) {
   if ((iter == NULL) || (iter->entry == NULL) || (ret_meta == NULL))
     return -1;
 
-  if (iter->entry->meta == NULL) {
-    *ret_meta = NULL;
-    return 0;
-  }
-
   *ret_meta = meta_data_clone(iter->entry->meta);
 
   return 0;

diff --git a/src/grpc.cc b/src/grpc.cc
index d465ffe..0f5cfec 100644 (file)
--- a/src/grpc.cc
+++ b/src/grpc.cc
@@ -56,6 +56,8 @@ using collectd::QueryValuesResponse;
 
 using google::protobuf::util::TimeUtil;
 
+typedef google::protobuf::Map<grpc::string, collectd::types::MetadataValue> grpcMetadata;
+
 /*
  * private types
  */
@@ -154,6 +156,124 @@ static grpc::Status unmarshal_ident(const collectd::types::Identifier &msg,
   return grpc::Status::OK;
 } /* unmarshal_ident() */
 
+static grpc::Status marshal_meta_data(meta_data_t *meta,
+                                      grpcMetadata *mutable_meta_data) {
+  char **meta_data_keys = nullptr;
+  int meta_data_keys_len = meta_data_toc(meta, &meta_data_keys);
+  if (meta_data_keys_len < 0) {
+    return grpc::Status(grpc::StatusCode::INTERNAL,
+                        grpc::string("error getting metadata keys"));
+  }
+
+  for (int i = 0; i < meta_data_keys_len; i++) {
+    char *key = meta_data_keys[i];
+    int md_type = meta_data_type(meta, key);
+
+    collectd::types::MetadataValue md_value;
+    md_value.Clear();
+
+    switch (md_type) {
+    case MD_TYPE_STRING:
+      char *md_string;
+      if (meta_data_get_string(meta, key, &md_string) != 0 || md_string == nullptr) {
+        strarray_free(meta_data_keys, meta_data_keys_len);
+        return grpc::Status(grpc::StatusCode::INTERNAL,
+                          grpc::string("missing metadata"));
+      }
+      md_value.set_string_value(md_string);
+      free(md_string);
+      break;
+    case MD_TYPE_SIGNED_INT:
+      int64_t int64_value;
+      if (meta_data_get_signed_int(meta, key, &int64_value) != 0) {
+        strarray_free(meta_data_keys, meta_data_keys_len);
+        return grpc::Status(grpc::StatusCode::INTERNAL,
+                          grpc::string("missing metadata"));
+      }
+      md_value.set_int64_value(int64_value);
+      break;
+    case MD_TYPE_UNSIGNED_INT:
+      uint64_t uint64_value;
+      if (meta_data_get_unsigned_int(meta, key, &uint64_value) != 0) {
+        strarray_free(meta_data_keys, meta_data_keys_len);
+        return grpc::Status(grpc::StatusCode::INTERNAL,
+                          grpc::string("missing metadata"));
+      }
+      md_value.set_uint64_value(uint64_value);
+      break;
+    case MD_TYPE_DOUBLE:
+      double double_value;
+      if (meta_data_get_double(meta, key, &double_value) != 0) {
+        strarray_free(meta_data_keys, meta_data_keys_len);
+        return grpc::Status(grpc::StatusCode::INTERNAL,
+                          grpc::string("missing metadata"));
+      }
+      md_value.set_double_value(double_value);
+      break;
+    case MD_TYPE_BOOLEAN:
+      bool bool_value;
+      if (meta_data_get_boolean(meta, key, &bool_value) != 0) {
+        strarray_free(meta_data_keys, meta_data_keys_len);
+        return grpc::Status(grpc::StatusCode::INTERNAL,
+                          grpc::string("missing metadata"));
+      }
+      md_value.set_bool_value(bool_value);
+      break;
+    default:
+      strarray_free(meta_data_keys, meta_data_keys_len);
+      ERROR("grpc: invalid metadata type (%d)", md_type);
+      return grpc::Status(grpc::StatusCode::INTERNAL,
+                          grpc::string("unknown metadata type"));
+    }
+
+    (*mutable_meta_data)[grpc::string(key)] = md_value;
+
+    strarray_free(meta_data_keys, meta_data_keys_len);
+  }
+
+  return grpc::Status::OK;
+}
+
+static grpc::Status unmarshal_meta_data(const grpcMetadata &rpc_metadata,
+                                        meta_data_t **md_out) {
+  *md_out = meta_data_create();
+  if (*md_out == nullptr) {
+    return grpc::Status(grpc::StatusCode::RESOURCE_EXHAUSTED,
+                        grpc::string("failed to metadata list"));
+  }
+  for (auto kv: rpc_metadata) {
+    auto k = kv.first.c_str();
+    auto v = kv.second;
+
+    // The meta_data collection individually allocates copies of the keys and
+    // string values for each entry, so it's safe for us to pass a reference
+    // to our short-lived strings.
+
+    switch (v.value_case()) {
+    case collectd::types::MetadataValue::ValueCase::kStringValue:
+      meta_data_add_string(*md_out, k, v.string_value().c_str());
+      break;
+    case collectd::types::MetadataValue::ValueCase::kInt64Value:
+      meta_data_add_signed_int(*md_out, k, v.int64_value());
+      break;
+    case collectd::types::MetadataValue::ValueCase::kUint64Value:
+      meta_data_add_unsigned_int(*md_out, k, v.uint64_value());
+      break;
+    case collectd::types::MetadataValue::ValueCase::kDoubleValue:
+      meta_data_add_double(*md_out, k, v.double_value());
+      break;
+    case collectd::types::MetadataValue::ValueCase::kBoolValue:
+      meta_data_add_boolean(*md_out, k, v.bool_value());
+      break;
+    default:
+      meta_data_destroy(*md_out);
+      return  grpc::Status(grpc::StatusCode::INVALID_ARGUMENT,
+                           grpc::string("Metadata of unknown type"));
+    }
+  }
+  return grpc::Status::OK;
+}
+
 static grpc::Status marshal_value_list(const value_list_t *vl,
                                        collectd::types::ValueList *msg) {
   auto id = msg->mutable_identifier();
@@ -172,78 +292,9 @@ static grpc::Status marshal_value_list(const value_list_t *vl,
 
   msg->clear_meta_data();
   if (vl->meta != nullptr) {
-    char **meta_data_keys = nullptr;
-    int meta_data_keys_len = meta_data_toc(vl->meta, &meta_data_keys);
-    if (meta_data_keys_len < 0) {
-      return grpc::Status(grpc::StatusCode::INTERNAL,
-                          grpc::string("error getting metadata keys"));
-    }
-
-    for (int i = 0; i < meta_data_keys_len; i++) {
-      char *key = meta_data_keys[i];
-      int md_type = meta_data_type(vl->meta, key);
-
-      collectd::types::MetadataValue md_value;
-      md_value.Clear();
-
-      switch (md_type) {
-      case MD_TYPE_STRING:
-        char *md_string;
-        if (meta_data_get_string(vl->meta, key, &md_string) != 0 || md_string == nullptr) {
-          strarray_free(meta_data_keys, meta_data_keys_len);
-          return grpc::Status(grpc::StatusCode::INTERNAL,
-                            grpc::string("missing metadata"));
-        }
-        md_value.set_string_value(md_string);
-        free(md_string);
-        break;
-      case MD_TYPE_SIGNED_INT:
-        int64_t int64_value;
-        if (meta_data_get_signed_int(vl->meta, key, &int64_value) != 0) {
-          strarray_free(meta_data_keys, meta_data_keys_len);
-          return grpc::Status(grpc::StatusCode::INTERNAL,
-                            grpc::string("missing metadata"));
-        }
-        md_value.set_int64_value(int64_value);
-        break;
-      case MD_TYPE_UNSIGNED_INT:
-        uint64_t uint64_value;
-        if (meta_data_get_unsigned_int(vl->meta, key, &uint64_value) != 0) {
-          strarray_free(meta_data_keys, meta_data_keys_len);
-          return grpc::Status(grpc::StatusCode::INTERNAL,
-                            grpc::string("missing metadata"));
-        }
-        md_value.set_uint64_value(uint64_value);
-        break;
-      case MD_TYPE_DOUBLE:
-        double double_value;
-        if (meta_data_get_double(vl->meta, key, &double_value) != 0) {
-          strarray_free(meta_data_keys, meta_data_keys_len);
-          return grpc::Status(grpc::StatusCode::INTERNAL,
-                            grpc::string("missing metadata"));
-        }
-        md_value.set_double_value(double_value);
-        break;
-      case MD_TYPE_BOOLEAN:
-        bool bool_value;
-        if (meta_data_get_boolean(vl->meta, key, &bool_value) != 0) {
-          strarray_free(meta_data_keys, meta_data_keys_len);
-          return grpc::Status(grpc::StatusCode::INTERNAL,
-                            grpc::string("missing metadata"));
-        }
-        md_value.set_bool_value(bool_value);
-        break;
-      default:
-        strarray_free(meta_data_keys, meta_data_keys_len);
-        return grpc::Status(grpc::StatusCode::INTERNAL,
-                            grpc::string("unknown metadata type"));
-      }
-
-      (*msg->mutable_meta_data())[grpc::string(key)] = md_value;
-
-      if (meta_data_keys != nullptr) {
-        strarray_free(meta_data_keys, meta_data_keys_len);
-      }
+    grpc::Status status = marshal_meta_data(vl->meta, msg->mutable_meta_data());
+    if (!status.ok()) {
+      return status;
     }
   }
 
@@ -286,41 +337,9 @@ static grpc::Status unmarshal_value_list(const collectd::types::ValueList &msg,
   if (!status.ok())
     return status;
 
-  vl->meta = meta_data_create();
-  if (vl->meta == nullptr) {
-    return grpc::Status(grpc::StatusCode::RESOURCE_EXHAUSTED,
-                        grpc::string("failed to metadata list"));
-  }
-  for (auto kv: msg.meta_data()) {
-    auto k = kv.first.c_str();
-    auto v = kv.second;
-
-    // The meta_data collection individually allocates copies of the keys and
-    // string values for each entry, so it's safe for us to pass a reference
-    // to our short-lived strings.
-
-    switch (v.value_case()) {
-    case collectd::types::MetadataValue::ValueCase::kStringValue:
-      meta_data_add_string(vl->meta, k, v.string_value().c_str());
-      break;
-    case collectd::types::MetadataValue::ValueCase::kInt64Value:
-      meta_data_add_signed_int(vl->meta, k, v.int64_value());
-      break;
-    case collectd::types::MetadataValue::ValueCase::kUint64Value:
-      meta_data_add_unsigned_int(vl->meta, k, v.uint64_value());
-      break;
-    case collectd::types::MetadataValue::ValueCase::kDoubleValue:
-      meta_data_add_double(vl->meta, k, v.double_value());
-      break;
-    case collectd::types::MetadataValue::ValueCase::kBoolValue:
-      meta_data_add_boolean(vl->meta, k, v.bool_value());
-      break;
-    default:
-      meta_data_destroy(vl->meta);
-      return  grpc::Status(grpc::StatusCode::INVALID_ARGUMENT,
-                           grpc::string("Metadata of unknown type"));
-    }
-  }
+  status = unmarshal_meta_data(msg.meta_data(), &vl->meta);
+  if (!status.ok())
+    return status;
 
   value_t *values = NULL;
   size_t values_len = 0;
@@ -366,9 +385,7 @@ static grpc::Status unmarshal_value_list(const collectd::types::ValueList &msg,
     vl->values_len = values_len;
   } else {
     meta_data_destroy(vl->meta);
-    if (values) {
-      free(values);
-    }
+    free(values);
   }
 
   return status;