[general] Revisit usage of memcpy everywhere

For copying structs, direct assignment is preferable to memcpy because
it is easier to read and gives more freedom to the compiler to be smart.

Using memcpy with strlen() as the size is the same as strcpy but with
an extra function call.

And sprintf is preferable to multiple strcats and memcpys to construct
a complex string.

Also made a few other simplifications along the way.

Signed-off-by: Geoff Gustafson <[email protected]>

Author: grgustaf

arc/src/arc_sensor.c

@@ -81,7 +81,7 @@ static void send_sensor_data(enum sensor_channel channel,
     msg.user_data = NULL;
     msg.error_code = ERROR_IPM_NONE;
     msg.data.sensor.channel = channel;
-    memcpy(&msg.data.sensor.reading, &reading, sizeof(union sensor_reading));
+    msg.data.sensor.reading = reading;
     ipm_send_msg(&msg);
 }
 

arc/src/main.c

@@ -70,7 +70,7 @@ static void queue_message(struct zjs_ipm_message *incoming_msg)
 
     if (msg != end_of_queue_ptr) {
         // copy the message into our queue to be process in the mainloop
-        memcpy(msg, incoming_msg, sizeof(struct zjs_ipm_message));
+        *msg = *incoming_msg;
     } else {
         // running out of space, disregard message
         ERR_PRINT("skipping incoming message\n");

src/ashell/comms-uart.c

@@ -532,5 +532,5 @@ void zjs_ashell_init()
 
 void comms_uart_set_config(struct comms_cfg_data *config)
 {
-    memcpy(&comms_config, config, sizeof(struct comms_cfg_data));
+    comms_config = *config;
 }

src/zjs_aio.c

@@ -127,9 +127,11 @@ static void ipm_msg_receive_callback(void *context, u32_t id,
 
     if (msg->flags & MSG_SYNC_FLAG) {
         zjs_ipm_message_t *result = (zjs_ipm_message_t *)msg->user_data;
+
         // synchronous ipm, copy the results
-        if (result)
-            memcpy(result, msg, sizeof(zjs_ipm_message_t));
+        if (result) {
+            *result = *msg;
+        }
 
         // un-block sync api
         k_sem_give(&aio_sem);

src/zjs_fs.c

@@ -159,7 +159,7 @@ static jerry_value_t create_stats_obj(struct fs_dirent *entry)
     if (!new_entry) {
         return zjs_error_context("malloc failed", 0, 0);
     }
-    memcpy(new_entry, entry, sizeof(struct fs_dirent));
+    *new_entry = *entry;
 
     jerry_set_object_native_pointer(stats_obj, new_entry, &stats_type_info);
 

src/zjs_grove_lcd_ipm.c

@@ -92,15 +92,17 @@ static void ipm_msg_receive_callback(void *context, u32_t id,
 
     if ((msg->flags & MSG_SYNC_FLAG) == MSG_SYNC_FLAG) {
         zjs_ipm_message_t *result = (zjs_ipm_message_t *)msg->user_data;
-        // synchrounus ipm, copy the results
-        if (result)
-            memcpy(result, msg, sizeof(zjs_ipm_message_t));
+
+        // synchronous ipm, copy the results
+        if (result) {
+            *result = *msg;
+        }
 
         // un-block sync api
         k_sem_give(&glcd_sem);
     } else {
         // asynchronous ipm, should not get here
-        ERR_PRINT("async message received\n");
+        ZJS_ASSERT(false, "async message received");
     }
 }
 

src/zjs_i2c_ipm_handler.c

@@ -43,15 +43,17 @@ static void ipm_msg_receive_callback(void *context, u32_t id,
 
     if (msg->flags & MSG_SYNC_FLAG) {
         zjs_ipm_message_t *result = (zjs_ipm_message_t *)msg->user_data;
+
         // synchronous ipm, copy the results
-        if (result)
-            memcpy(result, msg, sizeof(zjs_ipm_message_t));
+        if (result) {
+            *result = *msg;
+        }
 
         // un-block sync api
         k_sem_give(&i2c_sem);
     } else {
         // asynchronous ipm, should not get here
-        ERR_PRINT("async message received\n");
+        ZJS_ASSERT(false, "async message received");
     }
 }
 

src/zjs_net.c

@@ -184,6 +184,29 @@ static struct k_mutex socket_mutex;
 #define NET_HOSTNAME_MAX            32
 #define SOCK_READ_BUF_SIZE          128
 
+// TODO: this could perhaps be reused in dgram/ws etc
+static void zjs_copy_sockaddr(struct sockaddr *dst, struct sockaddr *src,
+                              socklen_t len)
+{
+    // requires: dst contains at least sizeof(sockaddr) bytes, src points to
+    //             a valid sockaddr struct (sockaddr_in or sockaddr_in6),
+    //             len is the address length if known, or 0
+    //  effects: copies src to dest, but only the number of bytes required by
+    //             the underlying address family; if len is given, asserts
+    //             that it matches the expected size
+    if (src->family == AF_INET) {
+        ZJS_ASSERT(!len || len == sizeof(sockaddr_in), "expected IPv4 length");
+        *(struct sockaddr_in *)dst = *(struct sockaddr_in *)src;
+    }
+    else if (src->family == AF_INET6) {
+        ZJS_ASSERT(!len || len == sizeof(sockaddr_in6), "expected IPv6 length");
+        *(struct sockaddr_in6 *)dst = *(struct sockaddr_in6 *)src;
+    }
+    else {
+        ZJS_ASSERT(false, "invalid sockaddr struct");
+    }
+}
+
 static int match_timer(sock_handle_t *sock, struct k_timer *timer)
 {
     FTRACE("sock = %p, timer = %p\n", sock, timer);
@@ -891,7 +914,7 @@ static void tcp_accepted(struct net_context *context,
     accept.context = context;
     accept.server_h = server_h;
     memset(&accept.addr, 0, sizeof(struct sockaddr));
-    memcpy(&accept.addr, addr, addrlen);
+    zjs_copy_sockaddr(&accept.addr, addr, addrlen);
 
     zjs_defer_work(accept_connection, &accept, sizeof(accept));
 }
@@ -1017,12 +1040,12 @@ static ZJS_DECL_FUNC(server_listen)
     double backlog = 0;
     u32_t size = NET_HOSTNAME_MAX;
     char hostname[size];
-    double family = 0;
+    u32_t family = 0;
 
     zjs_obj_get_double(argv[0], "port", &port);
     zjs_obj_get_double(argv[0], "backlog", &backlog);
     zjs_obj_get_string(argv[0], "host", hostname, size);
-    zjs_obj_get_double(argv[0], "family", &family);
+    zjs_obj_get_uint32(argv[0], "family", &family);
 
     // FIXME: validate or fix input, e.g. family
 
@@ -1032,41 +1055,32 @@ static ZJS_DECL_FUNC(server_listen)
 
     struct sockaddr addr;
     memset(&addr, 0, sizeof(struct sockaddr));
+    addr.family = (family == 6) ? AF_INET6 : AF_INET;  // default to IPv4
 
-    // default to IPv4
-    if (family == 0 || family == 4) {
-        family = 4;
-        CHECK(net_context_get(AF_INET, SOCK_STREAM, IPPROTO_TCP,
-                              &server_h->server_ctx));
+    CHECK(net_context_get(addr.family, SOCK_STREAM, IPPROTO_TCP,
+                          &server_h->server_ctx));
 
+    u32_t addrlen;
+    if (addr.family == AF_INET) {
         struct sockaddr_in *addr4 = (struct sockaddr_in *)&addr;
-
-        addr4->sin_family = AF_INET;
         addr4->sin_port = htons((int)port);
-
         net_addr_pton(AF_INET, hostname, &addr4->sin_addr);
+        addrlen = sizeof(struct sockaddr_in);
     } else {
-        CHECK(net_context_get(AF_INET6, SOCK_STREAM, IPPROTO_TCP,
-                              &server_h->server_ctx));
-
         struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)&addr;
-
-        addr6->sin6_family = AF_INET6;
         addr6->sin6_port = htons((int)port);
-
         net_addr_pton(AF_INET6, hostname, &addr6->sin6_addr);
+        addrlen = sizeof(struct sockaddr_in6);
     }
 
-    CHECK(net_context_bind(server_h->server_ctx, &addr,
-                           sizeof(struct sockaddr)));
+    CHECK(net_context_bind(server_h->server_ctx, &addr, addrlen));
     CHECK(net_context_listen(server_h->server_ctx, (int)backlog));
 
     server_h->listening = 1;
     server_h->port = (u16_t)port;
 
-    memcpy(&server_h->local, zjs_net_config_get_ip(server_h->server_ctx),
-           sizeof(struct sockaddr));
-    server_h->local = *zjs_net_config_get_ip(server_h->server_ctx);
+    zjs_copy_sockaddr(&server_h->local,
+                      zjs_net_config_get_ip(server_h->server_ctx), 0);
     zjs_obj_add_boolean(this, "listening", true);
 
     // Here we defer just to keep the call stack short; this is unless we

src/zjs_ocf_client.c

@@ -56,8 +56,6 @@ struct ocf_handler {
 
 static struct client_resource *resource_list = NULL;
 
-#define MAX_URI_LENGTH (30)
-
 static struct ocf_handler *new_ocf_handler(struct client_resource *res)
 {
     struct ocf_handler *h = zjs_malloc(sizeof(struct ocf_handler));
@@ -108,25 +106,14 @@ static jerry_value_t make_ocf_error(const char *name, const char *msg,
  */
 static char *create_url(const char *uuid, const char *path)
 {
-    //    oic://<uuid>/<path>
-    char *url = zjs_malloc(strlen(uuid) + strlen(path) + 8);
-    int count = 0;
-    url[0] = 'o';
-    url[1] = 'i';
-    url[2] = 'c';
-    url[3] = ':';
-    url[4] = '/';
-    url[5] = '/';
-    count = 6;
-    memcpy(url + count, uuid, strlen(uuid));
-    count += strlen(uuid);
-    if (path[0] != '/') {
-        url[count] = '/';
-        count++;
+    // create URL of form oic://<uuid>/<path>
+    if (path[0] == '/') {
+        path++;
     }
-    memcpy(url + count, path, strlen(path));
-    count += strlen(path);
-    url[count] = '\0';
+
+    int url_len = strlen(uuid) + strlen(path) + 8;
+    char *url = zjs_malloc(url_len);
+    snprintf(url, url_len, "oic://%s/%s", uuid, path);
     return url;
 }
 
@@ -388,20 +375,13 @@ static void add_resource(char *id, char *type, char *path,
     new->state = RES_STATE_SEARCHING;
 
     if (id) {
-        // FIXME: use strcpy instead of calling strlen 3x
-        new->device_id = zjs_malloc(strlen(id) + 1);
-        memcpy(new->device_id, id, strlen(id));
-        new->device_id[strlen(id)] = '\0';
+        new->device_id = zjs_alloc_from_string(id, NULL);
     }
     if (type) {
-        new->resource_type = zjs_malloc(strlen(type) + 1);
-        memcpy(new->resource_type, type, strlen(type));
-        new->resource_type[strlen(type)] = '\0';
+        new->resource_type = zjs_alloc_from_string(type, NULL);
     }
     if (path) {
-        new->resource_path = zjs_malloc(strlen(path) + 1);
-        memcpy(new->resource_path, path, strlen(path));
-        new->resource_path[strlen(path)] = '\0';
+        new->resource_path = zjs_alloc_from_string(path, NULL);
     }
 
     if (jerry_value_is_function(listener)) {
@@ -446,8 +426,6 @@ static oc_discovery_flags_t discovery(const char *di,
 {
     struct ocf_handler *h = (struct ocf_handler *)user_handle;
     int i;
-    int uri_len = strlen(uri);
-    uri_len = (uri_len >= MAX_URI_LENGTH) ? MAX_URI_LENGTH - 1 : uri_len;
 
     for (i = 0; i < oc_string_array_get_allocated_size(types); i++) {
         char *t = oc_string_array_get_item(types, i);
@@ -489,14 +467,13 @@ static oc_discovery_flags_t discovery(const char *di,
             cur->endpoint = endpoint;
 
             if (!cur->device_id) {
-                cur->device_id = zjs_malloc(strlen(di) + 1);
-                memcpy(cur->device_id, di, strlen(di));
-                cur->device_id[strlen(di)] = '\0';
+                cur->device_id = zjs_alloc_from_string(di, NULL);
             }
             if (!cur->resource_path) {
-                cur->resource_path = zjs_malloc(strlen(uri) + 1);
-                memcpy(cur->resource_path, uri, uri_len);
-                cur->resource_path[uri_len] = '\0';
+                // TODO: before this code was pretending to limit things to a
+                //   MAX_URI_LENGTH of 30, but it wasn't working right anyway;
+                //   not sure if there was a point to that
+                cur->resource_path = zjs_alloc_from_string(uri, NULL);
             }
             /*
              * Add the array of resource types to newly discovered resource

src/zjs_ocf_common.c

@@ -47,20 +47,17 @@ static bool ocf_foreach_prop(const jerry_value_t prop_name,
 {
     struct props_handle *handle = (struct props_handle *)data;
 
-    ZJS_GET_STRING(prop_name, name, OCF_MAX_PROP_NAME_LEN);
+    jerry_size_t maxlen = OCF_MAX_PROP_NAME_LEN;
+    char *name = zjs_alloc_from_jstring(prop_name, &maxlen);
 
-    /*
-     * Use zjs_alloc_from_jstring
-     */
     // Skip id/resourcePath/resourceType because that is not a resource property
     // that is sent out
-    if ((!strequal(name, "deviceId")) &&
-        (!strequal(name, "resourcePath")) &&
-        (!strequal(name, "resourceType"))) {
-        handle->names_array[handle->size] =
-            zjs_malloc(jerry_get_string_size(prop_name) + 1);
-        memcpy(handle->names_array[handle->size], name, strlen(name));
-        handle->names_array[handle->size][strlen(name)] = '\0';
+    char *props[] = { "deviceId", "resourcePath", "resourceType", NULL };
+    if (zjs_str_matches(name, props)) {
+        zjs_free(name);
+    }
+    else {
+        handle->names_array[handle->size] = name;
         ZVAL ret = jerry_set_property_by_index(handle->props_array,
                                                handle->size++, prop_value);
         if (jerry_value_has_error_flag(ret)) {