fix(coverity): Fix errors reported by Coverity

`bundle_array_copy()` has been implemented now, so `utarray_push_back()` can copy the whole `bundle` object to the last element of the bundle_array object. Therefore, we can use `bundle_transaction_free()` instead of using `free()` on `bundle_transaction_t` object. Memory leaks and the statement that execution can't reach are solved, too.
DLTcollab · May 15, 2020 · 1eb27ba · 1eb27ba
1 parent c63558b
commit 1eb27ba
Show file tree

Hide file tree

Showing 10 changed files with 57 additions and 28 deletions.
diff --git a/accelerator/core/core.c b/accelerator/core/core.c
@@ -525,7 +525,7 @@ status_t ta_get_bundles_by_addr(const iota_client_service_t* const service, tryt
 
       bundle_array_add(bundle_array, bundle);
       hash243_set_add(&bundle_hash_set, transaction_bundle(curr_tx));
-      free(bundle);
+      bundle_transactions_free(&bundle);
     }
   }
   hash243_set_free(&bundle_hash_set);
@@ -661,8 +661,8 @@ status_t broadcast_buffered_txn(const ta_core_t* const core) {
    *    Delete UUID-sent_transaction pair from key-value storage
    */
 
-  get_trytes_req_t* req;
-  get_trytes_res_t* res;
+  get_trytes_req_t* req = NULL;
+  get_trytes_res_t* res = NULL;
   do {
     char uuid[UUID_STR_LEN];
     flex_trit_t req_txn_flex_trits[NUM_FLEX_TRITS_SERIALIZED_TRANSACTION + 1];

diff --git a/accelerator/core/mam_core.c b/accelerator/core/mam_core.c
@@ -235,10 +235,15 @@ static status_t create_channel_fetch_all_transactions(const iota_client_service_
   status_t ret = SC_OK;
   find_transactions_req_t *txn_req = find_transactions_req_new();
   transaction_array_t *obj_res = transaction_array_new();
+  if (!txn_req || !obj_res) {
+    ret = SC_MAM_NULL;
+    ta_log_error("%s\n", ta_error_to_string(ret));
+    goto done;
+  }
 
   if (mam_api_channel_create(api, channel_depth, chid) != RC_OK) {
     ret = SC_MAM_FAILED_CREATE_OR_GET_ID;
-    ta_log_error("%s\n", "SC_MAM_FAILED_CREATE_OR_GET_ID");
+    ta_log_error("%s\n", ta_error_to_string(ret));
     goto done;
   }
 

diff --git a/accelerator/core/serializer/serializer.c b/accelerator/core/serializer/serializer.c
@@ -790,7 +790,7 @@ static status_t send_mam_message_mam_v1_req_deserialize(cJSON const* const json_
   json_value = cJSON_GetObjectItemCaseSensitive(json_key, "message");
   if (json_value != NULL) {
     size_t payload_size = strlen(json_value->valuestring);
-    char* payload = (char*)malloc((payload_size + 1) * sizeof(char));
+    data->message = (char*)malloc((payload_size + 1) * sizeof(char));
 
     // In case the payload is unicode, the character whose ASCII code is beyond 128 result to an
     // error status_t code
@@ -801,9 +801,7 @@ static status_t send_mam_message_mam_v1_req_deserialize(cJSON const* const json_
         goto done;
       }
     }
-
-    snprintf(payload, payload_size + 1, "%s", json_value->valuestring);
-    data->message = payload;
+    memcpy(data->message, json_value->valuestring, payload_size + 1);
   } else {
     ta_log_error("%s\n", ta_error_to_string(SC_SERIALIZER_NULL));
     ret = SC_SERIALIZER_NULL;
@@ -933,49 +931,55 @@ static status_t recv_mam_message_mam_v1_req_deserialize(cJSON const* const json_
   if (cJSON_HasObjectItem(json_key, "bundle_hash")) {
     json_value = cJSON_GetObjectItemCaseSensitive(json_key, "bundle_hash");
     if (json_value == NULL) {
-      ta_log_error("%s\n", ta_error_to_string(SC_CCLIENT_JSON_KEY));
-      return SC_CCLIENT_JSON_KEY;
+      ret = SC_CCLIENT_JSON_KEY;
+      ta_log_error("%s\n", ta_error_to_string(ret));
+      return ret;
     }
     if (cJSON_IsString(json_value) && (json_value->valuestring != NULL) &&
         (strlen(json_value->valuestring) == NUM_TRYTES_HASH)) {
       bundle_hash = (char*)malloc(sizeof(char) * (NUM_TRYTES_HASH + 1));
       strncpy(bundle_hash, json_value->valuestring, sizeof(char) * (NUM_TRYTES_HASH + 1));
     } else {
-      ta_log_error("%s\n", ta_error_to_string(SC_CCLIENT_JSON_PARSE));
-      return SC_CCLIENT_JSON_PARSE;
+      ret = SC_CCLIENT_JSON_PARSE;
+      ta_log_error("%s\n", ta_error_to_string(ret));
+      return ret;
     }
     goto set_data_id;
   }
 
   if (cJSON_HasObjectItem(json_key, "chid")) {
     json_value = cJSON_GetObjectItemCaseSensitive(json_key, "chid");
     if (json_value == NULL) {
-      ta_log_error("%s\n", ta_error_to_string(SC_CCLIENT_JSON_KEY));
-      return SC_CCLIENT_JSON_KEY;
+      ret = SC_CCLIENT_JSON_KEY;
+      ta_log_error("%s\n", ta_error_to_string(ret));
+      return ret;
     }
     if (cJSON_IsString(json_value) &&
         (json_value->valuestring != NULL && (strlen(json_value->valuestring) == NUM_TRYTES_HASH))) {
       chid = (char*)malloc(sizeof(char) * (NUM_TRYTES_HASH + 1));
       strncpy(chid, json_value->valuestring, sizeof(char) * (NUM_TRYTES_HASH + 1));
     } else {
-      ta_log_error("%s\n", ta_error_to_string(SC_CCLIENT_JSON_PARSE));
-      return SC_CCLIENT_JSON_PARSE;
+      ret = SC_CCLIENT_JSON_PARSE;
+      ta_log_error("%s\n", ta_error_to_string(ret));
+      return ret;
     }
   }
 
   if (cJSON_HasObjectItem(json_key, "msg_id")) {
     json_value = cJSON_GetObjectItemCaseSensitive(json_key, "msg_id");
     if (json_value == NULL) {
-      ta_log_error("%s\n", ta_error_to_string(SC_CCLIENT_JSON_KEY));
-      return SC_CCLIENT_JSON_KEY;
+      ret = SC_CCLIENT_JSON_KEY;
+      ta_log_error("%s\n", ta_error_to_string(ret));
+      return ret;
     }
     if (cJSON_IsString(json_value) && (json_value->valuestring != NULL) &&
         (strlen(json_value->valuestring) == NUM_TRYTES_MAM_MSG_ID)) {
       msg_id = (char*)malloc(sizeof(char) * (NUM_TRYTES_MAM_MSG_ID + 1));
       strncpy(msg_id, json_value->valuestring, sizeof(char) * (NUM_TRYTES_MAM_MSG_ID + 1));
     } else {
-      ta_log_error("%s\n", ta_error_to_string(SC_CCLIENT_JSON_PARSE));
-      return SC_CCLIENT_JSON_PARSE;
+      ret = SC_CCLIENT_JSON_PARSE;
+      ta_log_error("%s\n", ta_error_to_string(ret));
+      return ret;
     }
   }
 

diff --git a/accelerator/main.c b/accelerator/main.c
@@ -96,7 +96,10 @@ int main(int argc, char* argv[]) {
   if (quiet_mode) {
     // Destroy logger when quiet mode is on
     logger_helper_release(logger_id);
-    logger_helper_destroy();
+    if (logger_helper_destroy() != RC_OK) {
+      ta_log_error("Destroying logger failed %s.\n", MAIN_LOGGER);
+      return EXIT_FAILURE;
+    }
   } else {
     http_logger_init();
     apis_logger_init();

diff --git a/accelerator/mqtt_main.c b/accelerator/mqtt_main.c
@@ -46,7 +46,10 @@ int main(int argc, char *argv[]) {
   if (quiet_mode) {
     // Destroy logger when quiet mode is on
     logger_helper_release(logger_id);
-    logger_helper_destroy();
+    if (logger_helper_destroy() != RC_OK) {
+      ta_log_error("Destroying logger failed %s.\n", CONN_MQTT_LOGGER);
+      return EXIT_FAILURE;
+    }
   } else {
     mqtt_utils_logger_init();
     mqtt_common_logger_init();

diff --git a/connectivity/mqtt/duplex_callback.c b/connectivity/mqtt/duplex_callback.c
@@ -53,6 +53,7 @@ static status_t mqtt_request_handler(mosq_config_t *cfg, char *subscribe_topic,
 
   status_t ret = SC_OK;
   char *json_result = NULL;
+  char *res_topic = NULL;
   char device_id[ID_LEN] = {0};
 
   // get the Device ID.
@@ -110,7 +111,7 @@ static status_t mqtt_request_handler(mosq_config_t *cfg, char *subscribe_topic,
 
   // Set response publishing topic with the topic we got message and the Device ID (client ID) we got in the message
   int res_topic_len = strlen(subscribe_topic) + 1 + ID_LEN + 1;
-  char *res_topic = (char *)malloc(res_topic_len);
+  res_topic = (char *)malloc(res_topic_len);
   snprintf(res_topic, res_topic_len, "%s/%s", subscribe_topic, device_id);
   ret = gossip_channel_set(cfg, NULL, NULL, res_topic);
   if (ret != SC_OK) {
@@ -126,6 +127,7 @@ static status_t mqtt_request_handler(mosq_config_t *cfg, char *subscribe_topic,
 
 done:
   free(json_result);
+  free(res_topic);
   return ret;
 }
 

diff --git a/connectivity/mqtt/mqtt_common.c b/connectivity/mqtt/mqtt_common.c
@@ -198,8 +198,9 @@ status_t cfg_add_topic(mosq_config_t *cfg, client_type_t client_type, char *topi
 
   if (client_type == client_pub || client_type == client_duplex) {
     cfg->pub_config->topic = strdup(topic);
-  } else if (client_type == client_duplex) {
-    cfg->pub_config->response_topic = strdup(topic);
+    if (client_type == client_duplex) {
+      cfg->pub_config->response_topic = strdup(topic);
+    }
   } else {
     cfg->sub_config->topic_count++;
     cfg->sub_config->topics = realloc(cfg->sub_config->topics, cfg->sub_config->topic_count * sizeof(char *));

diff --git a/storage/scylladb_identity.c b/storage/scylladb_identity.c
@@ -135,9 +135,10 @@ status_t db_set_identity_hash(db_identity_t* obj, const cass_byte_t* hash, size_
   }
   if (hash == NULL) {
     ta_log_error("NULL pointer to hash to insert into identity table\n");
+    return SC_TA_NULL;
   }
   if (length != DB_NUM_TRYTES_HASH) {
-    ta_log_error("SC_STORAGE_INVALID_INPUT\n");
+    ta_log_error("%s\n", ta_error_to_string(SC_STORAGE_INVALID_INPUT));
     return SC_STORAGE_INVALID_INPUT;
   }
   memcpy(obj->hash, hash, DB_NUM_TRYTES_HASH);

diff --git a/storage/scylladb_identity.h b/storage/scylladb_identity.h
@@ -51,8 +51,8 @@ db_identity_array_t* db_identity_array_new();
 static inline void db_identity_array_free(db_identity_array_t** const db_identity_array) {
   if (db_identity_array && *db_identity_array) {
     utarray_free(*db_identity_array);
+    *db_identity_array = NULL;
   }
-  *db_identity_array = NULL;
 }
 
 /**

diff --git a/utils/bundle_array.h b/utils/bundle_array.h
@@ -24,8 +24,18 @@ extern "C" {
  */
 
 typedef UT_array bundle_array_t;
+// We should synchronize this implementation as to the implementation in the IOTA library
+static UT_icd bundle_transactions_icd = {sizeof(iota_transaction_t), 0, 0, 0};
+
+static inline void bundle_array_copy(void *_dst, const void *_src) {
+  bundle_transactions_t *bdl_dst = (bundle_transactions_t *)_dst;
+  bundle_transactions_t *bdl_src = (bundle_transactions_t *)_src;
+  iota_transaction_t *txn = NULL;
+  utarray_init(bdl_dst, &bundle_transactions_icd);
+  BUNDLE_FOREACH(bdl_src, txn) { bundle_transactions_add(bdl_dst, txn); }
+}
 
-static UT_icd bundle_array_icd = {sizeof(bundle_transactions_t), 0, 0, 0};
+static UT_icd bundle_array_icd = {sizeof(bundle_transactions_t), 0, bundle_array_copy, 0};
 
 /**
  * Allocate memory of bundle_array_t