Allocate data buffer for i2c read/write 15/128415/3
authorHyeongsik Min <hyeongsik.min@samsung.com>
Tue, 9 May 2017 23:52:55 +0000 (08:52 +0900)
committerHyeongsik Min <hyeongsik.min@samsung.com>
Fri, 12 May 2017 01:40:55 +0000 (10:40 +0900)
Instead of using fixed size buffer, allocate data buffer per i2c
session.

Change-Id: I1157aabcdbfb2bf1385e78e60cce4b968006253d
Signed-off-by: Hyeongsik Min <hyeongsik.min@samsung.com>
src/daemon/peripheral_bus.c
src/daemon/peripheral_bus.h
src/daemon/peripheral_bus_i2c.c
src/daemon/peripheral_bus_i2c.h
src/daemon/peripheral_common.h

index bc943e0..5e9660c 100644 (file)
@@ -217,22 +217,9 @@ gboolean handle_i2c_read(
 {
        peripheral_error_e ret = PERIPHERAL_ERROR_NONE;
        pb_i2c_data_h i2c_handle = GUINT_TO_POINTER(handle);
-       // TODO: Fix size of data buffer
-       unsigned char data[128];
-       GVariantBuilder *builder;
-       GVariant *data_array;
-       int i = 0;
-
-       ret = peripheral_bus_i2c_read(invocation, i2c_handle, length, data, user_data);
-
-       builder = g_variant_builder_new(G_VARIANT_TYPE("a(y)"));
-
-       for (i = 0; i < length; i++)
-               g_variant_builder_add(builder, "(y)", data[i]);
-       g_variant_builder_add(builder, "(y)", 0x00);
-       data_array = g_variant_new("a(y)", builder);
-       g_variant_builder_unref(builder);
+       GVariant *data_array = NULL;
 
+       ret = peripheral_bus_i2c_read(invocation, i2c_handle, length, &data_array, user_data);
        peripheral_io_gdbus_i2c_complete_read(i2c, invocation, data_array, ret);
 
        return true;
@@ -248,20 +235,8 @@ gboolean handle_i2c_write(
 {
        peripheral_error_e ret = PERIPHERAL_ERROR_NONE;
        pb_i2c_data_h i2c_handle = GUINT_TO_POINTER(handle);
-       // TODO: Fix size of data buffer
-       unsigned char data[128];
-       GVariantIter *iter;
-       guchar str;
-       int i = 0;
-
-       g_variant_get(data_array, "a(y)", &iter);
-       while (g_variant_iter_loop(iter, "(y)", &str)) {
-               data[i++] = str;
-               if (i == length || i >= 128) break;
-       }
-       g_variant_iter_free(iter);
 
-       ret = peripheral_bus_i2c_write(invocation, i2c_handle, length, (unsigned char*)data, user_data);
+       ret = peripheral_bus_i2c_write(invocation, i2c_handle, length, data_array, user_data);
        peripheral_io_gdbus_i2c_complete_write(i2c, invocation, ret);
 
        return true;
index 73df3d8..cba2634 100644 (file)
@@ -55,9 +55,13 @@ typedef struct {
 } peripheral_bus_gpio_data_s;
 
 typedef struct {
+       /* i2c device information */
        int bus;
        int address;
        int fd;
+       /* data buffer */
+       uint8_t *buffer;
+       int buffer_size;
        /* client info */
        pb_client_info_s *client_info;
 } peripheral_bus_i2c_data_s;
index f252c9e..c9dce59 100644 (file)
@@ -24,6 +24,9 @@
 #include "peripheral_bus.h"
 #include "peripheral_common.h"
 
+#define INITIAL_BUFFER_SIZE 128
+#define MAX_BUFFER_SIZE 8192
+
 static pb_client_info_s* peripheral_bus_i2c_get_client_info(GDBusMethodInvocation *invocation, peripheral_bus_s *pb_data)
 {
        pb_client_info_s *client_info;
@@ -101,22 +104,24 @@ static int peripheral_bus_i2c_data_free(pb_i2c_data_h i2c_handle, GList **list)
 {
        GList *i2c_list = *list;
        GList *link;
-       pb_i2c_data_h i2c_data;
+       pb_i2c_data_h i2c;
 
-       if (i2c_list == NULL)
+       if (i2c_handle == NULL)
                return -1;
 
        link = i2c_list;
        while (link) {
-               i2c_data = (pb_i2c_data_h)link->data;
+               i2c = (pb_i2c_data_h)link->data;
 
-               if (i2c_data == i2c_handle) {
+               if (i2c == i2c_handle) {
                        *list = g_list_remove_link(i2c_list, link);
-                       if (i2c_data->client_info) {
-                               free(i2c_data->client_info->id);
-                               free(i2c_data->client_info);
+                       if (i2c->client_info) {
+                               free(i2c->client_info->id);
+                               free(i2c->client_info);
                        }
-                       free(i2c_data);
+                       if (i2c->buffer)
+                               free(i2c->buffer);
+                       free(i2c);
                        g_list_free(link);
                        return 0;
                }
@@ -152,6 +157,15 @@ int peripheral_bus_i2c_open(GDBusMethodInvocation *invocation, int bus, int addr
        i2c_handle->bus = bus;
        i2c_handle->address = address;
        i2c_handle->client_info = peripheral_bus_i2c_get_client_info(invocation, pb_data);
+
+       i2c_handle->buffer = malloc(INITIAL_BUFFER_SIZE);
+       if (!(i2c_handle->buffer)) {
+               i2c_close(fd);
+               _E("Failed to allocate data buffer");
+               return PERIPHERAL_ERROR_OUT_OF_MEMORY;
+       }
+       i2c_handle->buffer_size = INITIAL_BUFFER_SIZE;
+
        *i2c = i2c_handle;
 
        _D("bus : %d, address : %d, pgid = %d, id = %s", bus, address,
@@ -161,32 +175,99 @@ int peripheral_bus_i2c_open(GDBusMethodInvocation *invocation, int bus, int addr
        return ret;
 }
 
-int peripheral_bus_i2c_read(GDBusMethodInvocation *invocation, pb_i2c_data_h i2c, int length, unsigned char *data, gpointer user_data)
+int peripheral_bus_i2c_read(GDBusMethodInvocation *invocation, pb_i2c_data_h i2c, int length, GVariant **data_array, gpointer user_data)
 {
        const gchar *id;
+       GVariantBuilder *builder;
+       int ret, i;
+
+       /* Handle validation */
+       RETVM_IF(!i2c || !(i2c->client_info), PERIPHERAL_ERROR_UNKNOWN, "i2c handle is not valid");
 
        id = g_dbus_method_invocation_get_sender(invocation);
 
+       /* Limit maximum length */
+       if (length > MAX_BUFFER_SIZE) length = MAX_BUFFER_SIZE;
+
+       /* Increase buffer if needed */
+       if (length > i2c->buffer_size) {
+               uint8_t *new;
+               new = (uint8_t*)realloc(i2c->buffer, length);
+
+               if (!new) {
+                       ret = PERIPHERAL_ERROR_OUT_OF_MEMORY;
+                       _E("Failed to realloc buffer");
+                       goto out;
+               }
+               i2c->buffer = new;
+               i2c->buffer_size = length;
+       }
+
        if (strcmp(i2c->client_info->id, id)) {
-               _E("Invalid access, handle id : %s, current id : %s", i2c->client_info->id, id);
-               return PERIPHERAL_ERROR_INVALID_OPERATION;
+               _E("Invalid access, handie id : %s, current id : %s", i2c->client_info->id, id);
+               ret = PERIPHERAL_ERROR_INVALID_OPERATION;
+               goto out;
        }
 
-       return i2c_read(i2c->fd, data, length);
+       ret = i2c_read(i2c->fd, i2c->buffer, length);
+
+       builder = g_variant_builder_new(G_VARIANT_TYPE("a(y)"));
+
+       for (i = 0; i < length; i++)
+               g_variant_builder_add(builder, "(y)", i2c->buffer[i]);
+       *data_array = g_variant_new("a(y)", builder);
+       g_variant_builder_unref(builder);
+
+       return ret;
+
+out:
+       builder = g_variant_builder_new(G_VARIANT_TYPE("a(y)"));
+
+       for (i = 0; i < i2c->buffer_size; i++)
+               g_variant_builder_add(builder, "(y)", 0x0);
+       *data_array = g_variant_new("a(y)", builder);
+       g_variant_builder_unref(builder);
+
+       return ret;
 }
 
-int peripheral_bus_i2c_write(GDBusMethodInvocation *invocation, pb_i2c_data_h i2c, int length, unsigned char *data, gpointer user_data)
+int peripheral_bus_i2c_write(GDBusMethodInvocation *invocation, pb_i2c_data_h i2c, int length, GVariant *data_array, gpointer user_data)
 {
        const gchar *id;
+       GVariantIter *iter;
+       guchar str;
+       int i = 0;
+
+       /* Handle validation */
+       RETVM_IF(!i2c || !(i2c->client_info), PERIPHERAL_ERROR_UNKNOWN, "i2c handle is not valid");
 
        id = g_dbus_method_invocation_get_sender(invocation);
 
-       if (strcmp(i2c->client_info->id, id)) {
-               _E("Invalid access, handle id : %s, current id : %s", i2c->client_info->id, id);
-               return PERIPHERAL_ERROR_INVALID_OPERATION;
+       RETVM_IF(strcmp(i2c->client_info->id, id),
+               PERIPHERAL_ERROR_INVALID_OPERATION,
+               "Invalid access, handie id : %s, current id : %s",
+               i2c->client_info->id, id);
+
+       /* Limit maximum length */
+       if (length > MAX_BUFFER_SIZE) length = MAX_BUFFER_SIZE;
+
+       /* Increase buffer if needed */
+       if (length > i2c->buffer_size) {
+               uint8_t *new;
+               new = (uint8_t*)realloc(i2c->buffer, length);
+
+               RETVM_IF(new == NULL, PERIPHERAL_ERROR_OUT_OF_MEMORY, "Failed to realloc buffer");
+               i2c->buffer = new;
+               i2c->buffer_size = length;
+       }
+
+       g_variant_get(data_array, "a(y)", &iter);
+       while (g_variant_iter_loop(iter, "(y)", &str) && i < length) {
+               i2c->buffer[i++] = str;
        }
+       g_variant_iter_free(iter);
 
-       return i2c_write(i2c->fd, data, length);
+       return i2c_write(i2c->fd, i2c->buffer, length);
 }
 
 int peripheral_bus_i2c_close(GDBusMethodInvocation *invocation, pb_i2c_data_h i2c, gpointer user_data)
@@ -195,14 +276,17 @@ int peripheral_bus_i2c_close(GDBusMethodInvocation *invocation, pb_i2c_data_h i2
        const gchar *id;
        int ret;
 
-       _D("i2c_close, bus : %d, address : %d, pgid = %d", i2c->bus, i2c->address, i2c->client_info->pgid);
+       /* Handle validation */
+       RETVM_IF(!i2c || !(i2c->client_info), PERIPHERAL_ERROR_UNKNOWN, "i2c handle is not valid");
+
+       _D("bus : %d, address : %d, pgid = %d", i2c->bus, i2c->address, i2c->client_info->pgid);
 
        id = g_dbus_method_invocation_get_sender(invocation);
 
-       if (strcmp(i2c->client_info->id, id)) {
-               _E("Invalid access, handle id : %s, current id : %s", i2c->client_info->id, id);
-               return PERIPHERAL_ERROR_INVALID_OPERATION;
-       }
+       RETVM_IF(strcmp(i2c->client_info->id, id),
+               PERIPHERAL_ERROR_INVALID_OPERATION,
+               "Invalid access, handle id : %s, current id : %s",
+               i2c->client_info->id, id);
 
        if ((ret = i2c_close(i2c->fd)) < 0)
                return ret;
index 6865ce7..c52bd39 100644 (file)
@@ -18,8 +18,8 @@
 #define __PERIPHERAL_BUS_I2C_H__
 
 int peripheral_bus_i2c_open(GDBusMethodInvocation *invocation, int bus, int address, pb_i2c_data_h *i2c, gpointer user_data);
-int peripheral_bus_i2c_read(GDBusMethodInvocation *invocation, pb_i2c_data_h i2c, int length, unsigned char *data, gpointer user_data);
-int peripheral_bus_i2c_write(GDBusMethodInvocation *invocation, pb_i2c_data_h i2c, int length, unsigned char *data, gpointer user_data);
+int peripheral_bus_i2c_read(GDBusMethodInvocation *invocation, pb_i2c_data_h i2c, int length, GVariant **data_array, gpointer user_data);
+int peripheral_bus_i2c_write(GDBusMethodInvocation *invocation, pb_i2c_data_h i2c, int length, GVariant *data_array, gpointer user_data);
 int peripheral_bus_i2c_close(GDBusMethodInvocation *invocation, pb_i2c_data_h i2c, gpointer user_data);
 
 #endif /* __PERIPHERAL_BUS_I2C_H__ */
index 905189f..9c3b8fe 100644 (file)
 #define _D(fmt, arg...) LOGD(fmt, ##arg)
 #define _W(fmt, arg...) LOGW(fmt, ##arg)
 
+#define RET_IF(expr) \
+       do { \
+               if (expr) { \
+                       _E("(%s)", #expr); \
+                       return; \
+               } \
+       } while (0)
+#define RETV_IF(expr, val) \
+       do {\
+               if (expr) { \
+                       _E("(%s)", #expr); \
+                       return (val); \
+               } \
+       } while (0)
+#define RETM_IF(expr, fmt, arg...) \
+       do {\
+               if (expr) { \
+                       _E(fmt, ##arg); \
+                       return; \
+               } \
+       } while (0)
+#define RETVM_IF(expr, val, fmt, arg...) \
+       do {\
+               if (expr) { \
+                       _E(fmt, ##arg); \
+                       return (val); \
+               } \
+       } while (0)
+
 #endif /* __PERIPHERAL_COMMON_H__ */