Fix issues reported by SonarCloud

CMakeLists.txt

@@ -822,7 +822,7 @@ if(BUILD_TESTING AND(UNIX OR MINGW))
 
     file(GLOB PLATFORMTEST_SRC port/unittest/*.cpp)
     file(MAKE_DIRECTORY ${CMAKE_BINARY_DIR}/storage_test)
-    package_add_test(platformtest ${PLATFORMTEST_SRC})
+    package_add_test(platformtest ${COMMONTEST_SRC} ${PLATFORMTEST_SRC})
 
     file(GLOB MESSAGINGTEST_SRC messaging/coap/unittest/*.cpp)
     package_add_test(messagingtest ${MESSAGINGTEST_SRC})

api/README.MD

@@ -0,0 +1,83 @@
+# Features
+
+## Dynamic and static allocation
+
+Whether dynamic or static allocation is enabled is determined by the macro `OC_DYNAMIC_ALLOCATION`.
+
+It effects the structures that are aliases of `struct oc_mmem` (`oc_handle_t`, `oc_string_t`, `oc_array_t`, `oc_string_array_t,` and `oc_byte_string_array_t`), which are allocated by `_oc_mmem_alloc` and deallocated by `_oc_mmem_free`.
+
+With dynamic allocation standard `malloc` and `free` calls are used.
+
+### Static allocation
+
+With static allocation preallocated static buffers are used.
+
+Allocation:
+
+* Take the desired number of bytes from the start of the unused part of the static buffer (if there are not enough available bytes in the buffer then `NULL` is returned)
+* Append the allocated variable to a global linked list of allocated variables
+
+Deallocation:
+
+* Reallocate data of all variables allocated after the variable currently being deallocated (ie they appear later in the global linked list) and write over the bytes previously used by the deallocated variable.
+* Increase the number of available bytes in the static buffer by the size of the deallocated variable.
+
+#### Pitfalls
+
+* Be careful when passing `oc_handle_t`, `oc_string_t`, `oc_array_t`, `oc_string_array_t,` and `oc_byte_string_array_t` by value.
+
+On allocation the pointer of the allocated variable is stored in the global linked list of allocations. If you copy a variable by value then pointer to this copy won't be in the global linked list and if you attempt a deallocation with this copy you will cause memory corruption.
+
+Invalid code:
+
+```C
+oc_string_t str1;
+oc_new_string(&str, "test", strlen("test"));
+
+oc_string_t str2 = str1;
+oc_free_string(&str2); // error, memory corruption
+
+```
+
+Valid code:
+
+```C
+oc_string_t str1;
+oc_new_string(&str, "test", strlen("test"));
+
+oc_string_t str2;
+oc_copy_string(&str2, &str1);
+```
+
+* Be careful when storing pointer to internal data of a `oc_handle_t`, `oc_string_t`, `oc_array_t`, `oc_string_array_t,` and `oc_byte_string_array_t` variable.
+
+If you store a pointer to internal data of a variable and then another variable, that has been allocated sooner is deallocated, then the stored pointer is invalidated.
+
+Invalid code:
+
+```C
+oc_string_t first;
+oc_new_string(&first, "first", strlen("first"));
+
+oc_string_t second;
+oc_new_string(&second, "second", strlen("second"));
+
+
+const char* second_str = oc_string(second);
+oc_free_string(&first); // second_str is now invalidated, because the variable second was allocated later and thus its internal data is reallocated after the variable first is deallocated
+
+printf("%s", second_str);
+```
+
+Valid code:
+
+```C
+oc_string_t first;
+oc_new_string(&first, "first", strlen("first"));
+
+oc_string_t second;
+oc_new_string(&second, "second", strlen("second"));
+
+oc_free_string(&first);
+printf("%s", oc_string(second));
+```

api/cloud/oc_cloud.c

@@ -78,19 +78,6 @@ cloud_manager_cb(oc_cloud_context_t *ctx)
   }
 }
 
-void
-cloud_set_string(oc_string_t *dst, const char *data, size_t len)
-{
-  if (oc_string(*dst)) {
-    oc_free_string(dst);
-  }
-  if (data && len) {
-    oc_new_string(dst, data, len);
-  } else {
-    memset(dst, 0, sizeof(*dst));
-  }
-}
-
 static oc_event_callback_retval_t
 start_manager(void *user_data)
 {
@@ -166,19 +153,18 @@ cloud_set_cloudconf(oc_cloud_context_t *ctx, const cloud_conf_update_t *data)
   assert(ctx != NULL);
   assert(data != NULL);
   if (data->auth_provider && data->auth_provider_len) {
-    cloud_set_string(&ctx->store.auth_provider, data->auth_provider,
-                     data->auth_provider_len);
+    oc_set_string(&ctx->store.auth_provider, data->auth_provider,
+                  data->auth_provider_len);
   }
   if (data->access_token && data->access_token_len) {
-    cloud_set_string(&ctx->store.access_token, data->access_token,
-                     data->access_token_len);
+    oc_set_string(&ctx->store.access_token, data->access_token,
+                  data->access_token_len);
   }
   if (data->ci_server && data->ci_server_len) {
-    cloud_set_string(&ctx->store.ci_server, data->ci_server,
-                     data->ci_server_len);
+    oc_set_string(&ctx->store.ci_server, data->ci_server, data->ci_server_len);
   }
   if (data->sid && data->sid_len) {
-    cloud_set_string(&ctx->store.sid, data->sid, data->sid_len);
+    oc_set_string(&ctx->store.sid, data->sid, data->sid_len);
   }
 }
 

api/cloud/oc_cloud_access.c

@@ -141,34 +141,33 @@ oc_cloud_access_register(oc_cloud_access_conf_t conf, const char *auth_provider,
   return oc_do_post();
 }
 
-oc_string_t
+void
 cloud_access_deregister_query(const char *uid, const char *access_token,
-                              size_t device)
+                              size_t device, oc_string_t *query)
 {
+  assert(query != NULL);
   oc_string_t q_uid;
   oc_concat_strings(&q_uid, "uid=", uid);
 
   char uuid[OC_UUID_LEN] = { 0 };
-  oc_uuid_to_str(oc_core_get_device_id(device), uuid, OC_UUID_LEN);
+  oc_uuid_to_str(oc_core_get_device_id(device), uuid, sizeof(uuid));
   oc_string_t q_di;
   oc_concat_strings(&q_di, "&di=", uuid);
   oc_string_t q_uid_di;
   oc_concat_strings(&q_uid_di, oc_string(q_uid), oc_string(q_di));
   oc_free_string(&q_uid);
   oc_free_string(&q_di);
 
-  oc_string_t q_uid_di_at;
   if (access_token != NULL) {
     oc_string_t q_at;
     oc_concat_strings(&q_at, "&accesstoken=", access_token);
-    oc_concat_strings(&q_uid_di_at, oc_string(q_uid_di), oc_string(q_at));
+    oc_concat_strings(query, oc_string(q_uid_di), oc_string(q_at));
     oc_free_string(&q_at);
   } else {
-    oc_new_string(&q_uid_di_at, oc_string(q_uid_di), oc_string_len(q_uid_di));
+    oc_new_string(query, oc_string(q_uid_di), oc_string_len(q_uid_di));
   }
 
   oc_free_string(&q_uid_di);
-  return q_uid_di_at;
 }
 
 bool
@@ -187,8 +186,8 @@ oc_cloud_access_deregister(oc_cloud_access_conf_t conf, const char *uid,
   }
 #endif /* OC_SECURITY */
 
-  oc_string_t query =
-    cloud_access_deregister_query(uid, access_token, conf.device);
+  oc_string_t query;
+  cloud_access_deregister_query(uid, access_token, conf.device, &query);
 
   bool s;
   if (conf.timeout > 0) {

api/cloud/oc_cloud_access_internal.h

@@ -34,11 +34,17 @@ extern "C" {
 /**
  * @brief Generate URI query for deregister request.
  *
- * @return URI query, must be freed by caller
+ * The format of the generated string is uid=${uid}&di={device
+ * uuid}&at=${access_token} or uid=${uid}&di={device uuid} based on whether
+ * access token is NULL or not.
+ *
+ * @param uid uid (cannot be NULL)
+ * @param access_token access token
+ * @param device device index
+ * @param query output variable (cannot be NULL)
  */
-oc_string_t cloud_access_deregister_query(const char *uid,
-                                          const char *access_token,
-                                          size_t device);
+void cloud_access_deregister_query(const char *uid, const char *access_token,
+                                   size_t device, oc_string_t *query);
 #ifdef __cplusplus
 }
 #endif

api/cloud/oc_cloud_context.c

@@ -177,7 +177,7 @@ cloud_context_has_permanent_access_token(const oc_cloud_context_t *ctx)
 void
 cloud_context_clear_access_token(oc_cloud_context_t *ctx)
 {
-  cloud_set_string(&ctx->store.access_token, NULL, 0);
+  oc_set_string(&ctx->store.access_token, NULL, 0);
   ctx->store.expires_in = 0;
 }
 

api/cloud/oc_cloud_deregister.c

@@ -157,8 +157,10 @@ check_accesstoken_for_deregister(oc_cloud_context_t *ctx)
 // to the request query if the resulting query size is within the limit.
 #define DEREGISTER_EMPTY_QUERY_HEADER_SIZE 38
 
-  oc_string_t query = cloud_access_deregister_query(
-    oc_string(ctx->store.uid), oc_string(ctx->store.access_token), ctx->device);
+  oc_string_t query;
+  cloud_access_deregister_query(oc_string(ctx->store.uid),
+                                oc_string(ctx->store.access_token), ctx->device,
+                                &query);
   size_t query_size = oc_string_len(query);
   oc_free_string(&query);
 

api/cloud/oc_cloud_internal.h

@@ -83,7 +83,6 @@ void cloud_reset_delayed_callback_ms(void *cb_data, oc_trigger_t callback,
                                      uint64_t milliseconds);
 
 void cloud_manager_cb(oc_cloud_context_t *ctx);
-void cloud_set_string(oc_string_t *dst, const char *data, size_t len);
 void cloud_set_last_error(oc_cloud_context_t *ctx, oc_cloud_error_t error);
 void cloud_set_cps(oc_cloud_context_t *ctx, oc_cps_t cps);
 void cloud_set_cps_and_last_error(oc_cloud_context_t *ctx, oc_cps_t cps,

api/cloud/oc_cloud_manager.c

@@ -256,10 +256,9 @@ cloud_manager_handle_register_response(oc_cloud_context_t *ctx,
     return false;
   }
 
-  cloud_set_string(&ctx->store.access_token, access_token, access_token_size);
-  cloud_set_string(&ctx->store.refresh_token, refresh_token,
-                   refresh_token_size);
-  cloud_set_string(&ctx->store.uid, uid, uid_size);
+  oc_set_string(&ctx->store.access_token, access_token, access_token_size);
+  oc_set_string(&ctx->store.refresh_token, refresh_token, refresh_token_size);
+  oc_set_string(&ctx->store.uid, uid, uid_size);
   ctx->store.expires_in = expires_in;
   if (ctx->store.expires_in > 0) {
     ctx->store.status |= OC_CLOUD_TOKEN_EXPIRY;
@@ -285,7 +284,7 @@ cloud_manager_handle_redirect_response(oc_cloud_context_t *ctx,
       memset(ctx->cloud_ep, 0, sizeof(oc_endpoint_t));
       ctx->cloud_ep_state = OC_SESSION_DISCONNECTED;
     }
-    cloud_set_string(&ctx->store.ci_server, value, size);
+    oc_set_string(&ctx->store.ci_server, value, size);
     return true;
   }
 
@@ -699,8 +698,8 @@ cloud_manager_handle_refresh_token_response(oc_cloud_context_t *ctx,
     return false;
   }
 
-  cloud_set_string(&ctx->store.access_token, access_value, access_size);
-  cloud_set_string(&ctx->store.refresh_token, refresh_value, refresh_size);
+  oc_set_string(&ctx->store.access_token, access_value, access_size);
+  oc_set_string(&ctx->store.refresh_token, refresh_value, refresh_size);
   ctx->store.expires_in = expires_in;
   if (ctx->store.expires_in > 0) {
     ctx->store.status |= OC_CLOUD_TOKEN_EXPIRY;

api/cloud/oc_cloud_store.c

@@ -189,38 +189,38 @@ cloud_store_parse_string_property(const oc_rep_t *rep, oc_cloud_store_t *store)
   assert(rep->type == OC_REP_STRING);
   if (oc_rep_is_property(rep, CLOUD_XSTR(CLOUD_CI_SERVER),
                          CLOUD_XSTRLEN(CLOUD_CI_SERVER))) {
-    cloud_set_string(&store->ci_server, oc_string(rep->value.string),
-                     oc_string_len(rep->value.string));
+    oc_set_string(&store->ci_server, oc_string(rep->value.string),
+                  oc_string_len(rep->value.string));
     return true;
   }
   if (oc_rep_is_property(rep, CLOUD_XSTR(CLOUD_SID),
                          CLOUD_XSTRLEN(CLOUD_SID))) {
-    cloud_set_string(&store->sid, oc_string(rep->value.string),
-                     oc_string_len(rep->value.string));
+    oc_set_string(&store->sid, oc_string(rep->value.string),
+                  oc_string_len(rep->value.string));
     return true;
   }
   if (oc_rep_is_property(rep, CLOUD_XSTR(CLOUD_AUTH_PROVIDER),
                          CLOUD_XSTRLEN(CLOUD_AUTH_PROVIDER))) {
-    cloud_set_string(&store->auth_provider, oc_string(rep->value.string),
-                     oc_string_len(rep->value.string));
+    oc_set_string(&store->auth_provider, oc_string(rep->value.string),
+                  oc_string_len(rep->value.string));
     return true;
   }
   if (oc_rep_is_property(rep, CLOUD_XSTR(CLOUD_UID),
                          CLOUD_XSTRLEN(CLOUD_UID))) {
-    cloud_set_string(&store->uid, oc_string(rep->value.string),
-                     oc_string_len(rep->value.string));
+    oc_set_string(&store->uid, oc_string(rep->value.string),
+                  oc_string_len(rep->value.string));
     return true;
   }
   if (oc_rep_is_property(rep, CLOUD_XSTR(CLOUD_ACCESS_TOKEN),
                          CLOUD_XSTRLEN(CLOUD_ACCESS_TOKEN))) {
-    cloud_set_string(&store->access_token, oc_string(rep->value.string),
-                     oc_string_len(rep->value.string));
+    oc_set_string(&store->access_token, oc_string(rep->value.string),
+                  oc_string_len(rep->value.string));
     return true;
   }
   if (oc_rep_is_property(rep, CLOUD_XSTR(CLOUD_REFRESH_TOKEN),
                          CLOUD_XSTRLEN(CLOUD_REFRESH_TOKEN))) {
-    cloud_set_string(&store->refresh_token, oc_string(rep->value.string),
-                     oc_string_len(rep->value.string));
+    oc_set_string(&store->refresh_token, oc_string(rep->value.string),
+                  oc_string_len(rep->value.string));
     return true;
   }
 
@@ -323,21 +323,21 @@ cloud_store_initialize(oc_cloud_store_t *store)
 {
   cloud_store_deinitialize(store);
 #define DEFAULT_CLOUD_CIS "coaps+tcp://127.0.0.1"
-  cloud_set_string(&store->ci_server, DEFAULT_CLOUD_CIS,
-                   strlen(DEFAULT_CLOUD_CIS));
+  oc_set_string(&store->ci_server, DEFAULT_CLOUD_CIS,
+                strlen(DEFAULT_CLOUD_CIS));
 #define DEFAULT_CLOUD_SID "00000000-0000-0000-0000-000000000000"
-  cloud_set_string(&store->sid, DEFAULT_CLOUD_SID, strlen(DEFAULT_CLOUD_SID));
+  oc_set_string(&store->sid, DEFAULT_CLOUD_SID, strlen(DEFAULT_CLOUD_SID));
 }
 
 void
 cloud_store_deinitialize(oc_cloud_store_t *store)
 {
-  cloud_set_string(&store->ci_server, NULL, 0);
-  cloud_set_string(&store->auth_provider, NULL, 0);
-  cloud_set_string(&store->uid, NULL, 0);
-  cloud_set_string(&store->access_token, NULL, 0);
-  cloud_set_string(&store->refresh_token, NULL, 0);
-  cloud_set_string(&store->sid, NULL, 0);
+  oc_set_string(&store->ci_server, NULL, 0);
+  oc_set_string(&store->auth_provider, NULL, 0);
+  oc_set_string(&store->uid, NULL, 0);
+  oc_set_string(&store->access_token, NULL, 0);
+  oc_set_string(&store->refresh_token, NULL, 0);
+  oc_set_string(&store->sid, NULL, 0);
   store->status = 0;
   store->expires_in = 0;
   store->cps = OC_CPS_UNINITIALIZED;