From 9172705f2887a4e32b5cdbffd065591a15ac9254 Mon Sep 17 00:00:00 2001 From: Hyeongsik Min Date: Wed, 10 May 2017 08:52:55 +0900 Subject: [PATCH] Allocate data buffer for i2c read/write Instead of using fixed size buffer, allocate data buffer per i2c session. Change-Id: I1157aabcdbfb2bf1385e78e60cce4b968006253d Signed-off-by: Hyeongsik Min --- src/daemon/peripheral_bus.c | 31 +--------- src/daemon/peripheral_bus.h | 4 ++ src/daemon/peripheral_bus_i2c.c | 128 +++++++++++++++++++++++++++++++++------- src/daemon/peripheral_bus_i2c.h | 4 +- src/daemon/peripheral_common.h | 29 +++++++++ 5 files changed, 144 insertions(+), 52 deletions(-) diff --git a/src/daemon/peripheral_bus.c b/src/daemon/peripheral_bus.c index bc943e0..5e9660c 100644 --- a/src/daemon/peripheral_bus.c +++ b/src/daemon/peripheral_bus.c @@ -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; diff --git a/src/daemon/peripheral_bus.h b/src/daemon/peripheral_bus.h index 73df3d8..cba2634 100644 --- a/src/daemon/peripheral_bus.h +++ b/src/daemon/peripheral_bus.h @@ -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; diff --git a/src/daemon/peripheral_bus_i2c.c b/src/daemon/peripheral_bus_i2c.c index f252c9e..c9dce59 100644 --- a/src/daemon/peripheral_bus_i2c.c +++ b/src/daemon/peripheral_bus_i2c.c @@ -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; diff --git a/src/daemon/peripheral_bus_i2c.h b/src/daemon/peripheral_bus_i2c.h index 6865ce7..c52bd39 100644 --- a/src/daemon/peripheral_bus_i2c.h +++ b/src/daemon/peripheral_bus_i2c.h @@ -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__ */ diff --git a/src/daemon/peripheral_common.h b/src/daemon/peripheral_common.h index 905189f..9c3b8fe 100644 --- a/src/daemon/peripheral_common.h +++ b/src/daemon/peripheral_common.h @@ -26,4 +26,33 @@ #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__ */ -- 2.7.4