Bug Fix to original behavior 10/130210/8
authorKunhoon Baik <knhoon.baik@samsung.com>
Fri, 19 May 2017 10:50:32 +0000 (19:50 +0900)
committerKichan Kwon <k_c.kwon@samsung.com>
Mon, 22 May 2017 02:04:11 +0000 (11:04 +0900)
During migration to gdbus, there are some bugs changing communication behaviors.
This patch fixes such bugs

In addition, following issues also are solved.
a) variable uninitilization for checking dbus param error
b) memory leak

Change-Id: I1f89cf761fced330cb90a24e823d31f0a68d74b5

src/common/dbus-handler.h
src/heart/heart-battery.c
src/heart/heart-cpu.c
src/heart/heart-memory.c
src/heart/heart-storage.c
src/heart/include/logging.h
src/heart/logging.c
src/proc-stat/proc-monitor.c
src/proc-stat/proc-usage-stats-helper.c
src/proc-stat/proc-usage-stats.c

index 1704029..ecb9436 100644 (file)
@@ -59,13 +59,15 @@ struct d_bus_signal {
        guint subscription_id;
 };
 
-//#define D_BUS_REPLY_NULL(ivc) g_dbus_method_invocation_return_value(ivc, g_variant_new_tuple(NULL, 0));
+//This code is better. However, this code needs to be verified.
+//#define D_BUS_REPLY_NULL(ivc) g_dbus_method_invocation_return_value(ivc, NULL);
 
 #define D_BUS_REPLY_NULL(ivc)  \
 {      \
        GDBusMessage *re = g_dbus_message_new_method_reply(g_dbus_method_invocation_get_message(ivc));  \
        d_bus_reply_message(re);        \
        g_object_unref(re);     \
+       g_object_unref(ivc);    \
 }
 
 #define D_BUS_REPLY_TIMEOUT  (120 * 1000)
index 6ff0783..cd53b13 100644 (file)
@@ -2019,7 +2019,7 @@ static void heart_battery_capacity_status(GVariant *params)
         * "f1ae1d1f270e9 battery: add battery capacity dbus signal broadcast")
         */
 
-       int capacity;
+       int capacity = -1;
 
        g_variant_get(params, "(i)", &capacity);
        if (capacity < 0) {
@@ -2057,7 +2057,7 @@ static void heart_battery_charger_status(GVariant *params)
         * 0 - charger was disconnected
         * 1 - charger was connected
         */
-       int charger_status, cap_history_size;
+       int charger_status = -1, cap_history_size;
 
        g_variant_get(params, "(i)", &charger_status);
        if (charger_status < 0) {
@@ -2304,7 +2304,7 @@ exit:
 
 static void dbus_get_battery_capacity_history(GDBusMethodInvocation *invocation, GVariant *params)
 {
-       int ret, size, period, index;
+       int ret, size, period = -1, index;
        struct heart_battery_capacity *lbc;
        GSList *iter;
        time_t curr = time(NULL);
@@ -2474,7 +2474,7 @@ static int get_battery_remaining_time(int mode, enum charger_status_type status)
 
 static void dbus_get_battery_remaining_time(GDBusMethodInvocation *invocation, GVariant *params)
 {
-       int ret, mode;
+       int ret, mode = -1;
 
        g_variant_get(params, "(i)", &mode);
        if (mode < 0) {
index a4de287..9b286c7 100644 (file)
@@ -1286,7 +1286,7 @@ unlock_exit:
 
 static void dbus_heart_get_cpu_data_list(GDBusMethodInvocation *invocation, GVariant *params)
 {
-       int period, index, i, ret;
+       int period = -1, index, i, ret;
        gpointer value;
        gpointer key;
        GHashTableIter h_iter;
@@ -1295,7 +1295,7 @@ static void dbus_heart_get_cpu_data_list(GDBusMethodInvocation *invocation, GVar
        char *appid;
        unsigned long utime, stime, ftime, total;
 
-       int uid;
+       int uid = -1;
        GHashTable *hashtable;
 
        GVariantBuilder builder, *sub_builder;
@@ -1431,16 +1431,10 @@ static void dbus_heart_update_cpu_data(GDBusMethodInvocation *invocation, GVaria
 static void dbus_heart_sync_cpu_data(GDBusMethodInvocation *invocation, GVariant *params)
 {
        int ret = 0;
-       GDBusMessage *reply = g_dbus_message_new_method_reply(
-                       g_dbus_method_invocation_get_message(invocation));
-       if (!reply) {
-               _E("Failed to make reply message");
-               return;
-       }
 
        heart_cpu_update_app_list(NULL);
 
-       ret = logging_sync(reply);
+       ret = logging_sync(invocation);
        if (ret)
                g_dbus_method_invocation_return_value(invocation, g_variant_new("(i)", ret));
 }
index da692e9..925acf2 100644 (file)
@@ -1158,7 +1158,7 @@ static gboolean heart_memory_notify(gpointer data)
 static void dbus_get_memory_latest(GDBusMethodInvocation *invocation, GVariant *params)
 {
        int ret;
-       char *appid;
+       char *appid = NULL;
        unsigned int pss = 0, uss = 0;
 
        g_variant_get(params, "(&s)", &appid);
@@ -1182,8 +1182,8 @@ static void dbus_get_memory_latest(GDBusMethodInvocation *invocation, GVariant *
 
 static void dbus_get_memory_data(GDBusMethodInvocation *invocation, GVariant *params)
 {
-       int period;
-       char *appid;
+       int period = -1;
+       char *appid = NULL;
        struct heart_memory_data *md;
 
        g_variant_get(params, "(&si)", &appid, &period);
@@ -1208,7 +1208,7 @@ static void dbus_get_memory_data(GDBusMethodInvocation *invocation, GVariant *pa
 
 static void dbus_get_memory_data_list(GDBusMethodInvocation *invocation, GVariant *params)
 {
-       int i, ret, period;
+       int i, ret, period = -1;
        char *appid, *pkgid;
        GArray *temp_array;
        GVariantBuilder builder, *sub_builder;
@@ -1254,7 +1254,7 @@ static void dbus_get_memory_data_list(GDBusMethodInvocation *invocation, GVarian
 
 static void dbus_get_memorydb(GDBusMethodInvocation *invocation, GVariant *params)
 {
-       int i, period;
+       int i, period = -1;
        char *appid, *pkgid;
        GArray *temp_array;
        GVariantBuilder builder, *sub_builder;
@@ -1295,7 +1295,7 @@ static void dbus_get_memorydb(GDBusMethodInvocation *invocation, GVariant *param
 
 static void dbus_get_memoryforeach(GDBusMethodInvocation *invocation, GVariant *params)
 {
-       int i, period;
+       int i, period = -1;
        char *appid, *pkgid;
        GArray *temp_array;
        GVariantBuilder builder, *sub_builder;
index a1d6a7c..9f6f1e6 100644 (file)
@@ -55,7 +55,7 @@ static GQueue *queue = NULL;
 static void dbus_insert_log(GDBusMethodInvocation *invocation, GVariant *params)
 {
        int ret;
-       int pid;
+       int pid = 0;
        char *pkgid = NULL;
        char *data = NULL;
 
index 3c82c61..1f31c17 100644 (file)
@@ -132,7 +132,7 @@ int logging_read_foreach(char *name, char *appid, char *pkgid,
 int logging_modify_appid(char *module_name, char *old_appid, char *new_appid, int pid);
 void logging_update(int force);
 void logging_save_to_storage(int force);
-int logging_sync(GDBusMessage *reply);
+int logging_sync(GDBusMethodInvocation *reply_invocation);
 int logging_leveldb_put(char *key, unsigned int key_len, char *value, unsigned int value_len);
 int logging_leveldb_putv(char *key, unsigned int key_len, const char *fmt, ...);
 int logging_leveldb_read(char *key, unsigned int key_len, char *value, unsigned int value_len);
index 1762d54..8314004 100644 (file)
@@ -136,7 +136,7 @@ static leveldb_writeoptions_t *woptions;
 
 static struct logging_object *logging_instance = NULL;
 
-static GDBusMessage *sync_reply;
+static GDBusMethodInvocation *sync_reply_invocation;
 
 time_t logging_get_time(int clk_id)
 {
@@ -1413,25 +1413,21 @@ static void *logging_sync_thread_main(void *arg)
        }
 
 reply:
-       g_dbus_message_set_body(sync_reply, g_variant_new("(i)", ret));
+       g_dbus_method_invocation_return_value(sync_reply_invocation, g_variant_new("(i)", ret));
 
-       if (d_bus_reply_message(sync_reply) != RESOURCED_ERROR_NONE)
-               _E("Failed to reply sync request");
-
-       g_object_unref(sync_reply);
        logging_sync_thread = 0;
 
        pthread_exit(NULL);
 }
 
-int logging_sync(GDBusMessage *reply)
+int logging_sync(GDBusMethodInvocation *reply_invocation)
 {
        if (logging_sync_thread) {
                _I("logging sync thread %u is already running", (unsigned)logging_sync_thread);
                return logging_sync_thread;
        }
 
-       sync_reply = reply;
+       sync_reply_invocation = reply_invocation;
 
        if (!pthread_create(&logging_sync_thread, NULL, (void *)logging_sync_thread_main, NULL))
                return 0;
index c78299b..4f455c8 100755 (executable)
@@ -420,7 +420,7 @@ static void proc_dbus_exclude_signal_handler(GVariant *params)
        int len;
 
        g_variant_get(params, "(&si)", &str, &pid);
-       if (!str || !pid == 0) {
+       if (!str || pid == 0) {
                _D("there is no message");
                return;
        }
@@ -572,12 +572,15 @@ static void proc_dbus_watchdog_result(GVariant *params)
        proc_watchdog.signum = -1;
 }
 
+#define RETRY_MAX 5
 static int proc_dbus_show_popup(const char *value)
 {
+       char str_val[32];       // The original max size of represented string is 32
        int i, ret_val = 0;
        char *args[4] = { WATCHDOG_KEY1, WATCHDOG_VALUE_1, WATCHDOG_KEY2 };
 
-       args[3] = strndup(value, strlen(value));
+       snprintf(str_val, sizeof(str_val), "%s", value);
+       args[3] = &(str_val[0]);
 
        i = 0;
 
@@ -587,9 +590,7 @@ static int proc_dbus_show_popup(const char *value)
                if (!ret_val)
                        break;
                _E("Re-try to sync DBUS message, err_count : %d", i);
-       } while (i++ < 5);
-
-       free(args[3]);
+       } while (i++ < RETRY_MAX);
 
        return ret_val;
 }
@@ -657,9 +658,13 @@ static void proc_dbus_watchdog_handler(GVariant *params)
 static void send_dump_signal(char *signal)
 {
        char *arg[1];
+       char tmpc;
+       int pid_len = 0; // should get the length of the PID decimal representation
+
        pid_t pid = getpid();
-       arg[0] = (char *)malloc(sizeof(pid_t));
-       snprintf(arg[0], sizeof(pid_t), "%u", pid);
+       pid_len = snprintf(&tmpc, 1, "%u", pid);
+       arg[0] = (char *)malloc(pid_len+1);
+       snprintf(arg[0], pid_len+1, "%u", pid);
 
        d_bus_broadcast_signal(DUMP_SERVICE_OBJECT_PATH,
            DUMP_SERVICE_INTERFACE_NAME, signal, "i", arg);
index 29f2d6b..8307e14 100644 (file)
@@ -236,5 +236,7 @@ void proc_free_runtime_info_task(struct runtime_info_task *rt_task)
        if (rt_task->pipe_fds[1] >= 0)
                close(rt_task->pipe_fds[1]);
 
+       // we assume that rt_task->task_invocation is already freed before calling proc_free_runtime_info_task.
+
        free(rt_task);
 }
index ff626ce..309c432 100644 (file)
@@ -104,6 +104,7 @@ static bool proc_runtime_info_task_cb(int fd, void *data)
                ret = proc_read_from_usage_struct(rt_task->usage_info_list, i, result, rt_task->task_type);
                if (ret != RESOURCED_ERROR_NONE) {
                        _E("task %s: error in reading from usage info struct", rt_task->task_name);
+                       g_variant_builder_unref(sub_builder);
                        goto error;
                }
 
@@ -134,15 +135,18 @@ send_message:
        reply = g_dbus_message_new_method_reply(g_dbus_method_invocation_get_message(rt_task->task_invocation));
        if (!reply) {
                _E("task %s: out of memory to allocate for reply gdbus message. not attempting again!!!", rt_task->task_name);
+               g_object_unref(rt_task->task_invocation);
                return false;
        }
        _D("task %s: sending reply dbus message", rt_task->task_name);
        g_dbus_message_set_body(reply, g_variant_builder_end(&builder));
+
        ret = d_bus_reply_message(reply);
        if (ret != RESOURCED_ERROR_NONE)
                _E("task %s: sending message failed. not attempting again!!!", rt_task->task_name);
 
        g_object_unref(reply);
+       g_object_unref(rt_task->task_invocation);
        proc_free_runtime_info_task(rt_task);
        return false;
 }
@@ -285,7 +289,7 @@ error:
        if (rt_task)
                _D("task %s: error occured, sending error reply message", rt_task->task_name);
 
-       D_BUS_REPLY_NULL(invocation);
+       g_dbus_method_invocation_return_value(invocation, g_variant_new("(i)", ret));
 
        proc_free_runtime_info_task(rt_task);
 }