[IMPROVE] use list for saving target binary 46/50746/1
authorAnatolii Nikulin <nikulin.a@samsung.com>
Thu, 29 Oct 2015 08:59:22 +0000 (11:59 +0300)
committerDmitry Kovalenko <d.kovalenko@samsung.com>
Fri, 30 Oct 2015 13:22:11 +0000 (06:22 -0700)
Dynamically changed array is more difficult to implement
and may cause memory errors.
Using the list instead of an array is preferable.

Change-Id: I20b5ce6be161db9b83a89080ff8dc75e99970651
Signed-off-by: Anatolii Nikulin <nikulin.a@samsung.com>
(cherry picked from commit c10e49a445762009aaed62d834d5f96ea6236973)

preload/preload_control.c

index 24c4546..fd2d773 100644 (file)
@@ -1,7 +1,8 @@
 #include <linux/mm.h>
 #include <linux/slab.h>
-#include <linux/mutex.h>
+#include <linux/rwsem.h>
 #include <linux/limits.h>
+#include <linux/list.h>
 
 #include <us_manager/sspt/ip.h>
 
 #include "preload_probe.h"
 #include "preload_module.h"
 
-
-#define DEFAULT_SLOTS_COUNT 5
-#define DEFAULT_SLOTS_STEP 2
-
 struct bin_desc {
+       struct list_head list;
        struct dentry *dentry;
        char *filename;
 };
 
-static struct bin_desc *target_binaries = NULL;
-static unsigned int target_binaries_cnt = 0;
-static unsigned int target_binaries_slots = 0;
-
-static DEFINE_MUTEX(__target_binaries_mutex);
-
-
-static inline void __target_binaries_lock(void)
-{
-       mutex_lock(&__target_binaries_mutex);
-}
-
-static inline void __target_binaries_unlock(void)
-{
-       mutex_unlock(&__target_binaries_mutex);
-}
+static LIST_HEAD(target_binaries_list);
+static DECLARE_RWSEM(target_binaries_lock);
+static int target_binaries_cnt = 0;
 
 static inline struct task_struct *__get_task_struct(void)
 {
        return current;
 }
 
-static int __alloc_target_binaries_no_lock(unsigned int cnt)
+static struct bin_desc *__alloc_target_binary(struct dentry *dentry,
+                                             char *name, int namelen)
 {
-       target_binaries = kmalloc(sizeof(*target_binaries) * cnt, GFP_KERNEL);
-       if (target_binaries == NULL)
-               return -ENOMEM;
+       struct bin_desc *p = NULL;
 
-       target_binaries_slots = cnt;
+       p = kzalloc(sizeof(*p), GFP_KERNEL);
+       if (!p)
+               return NULL;
 
-       return 0;
+       INIT_LIST_HEAD(&p->list);
+       p->filename = kmalloc(namelen + 1, GFP_KERNEL);
+       if (!p->filename)
+               goto fail;
+       memcpy(p->filename, name, namelen);
+       p->filename[namelen] = '\0';
+       p->dentry = dentry;
+
+       return p;
+fail:
+       kfree(p);
+       return NULL;
 }
 
-static int __alloc_target_binaries(unsigned int cnt)
+static void __free_target_binary(struct bin_desc *p)
 {
-       int ret;
-
-       __target_binaries_lock();
-       ret = __alloc_target_binaries_no_lock(cnt);
-       __target_binaries_unlock();
-
-       return ret;
+       kfree(p->filename);
+       kfree(p);
 }
 
 static void __free_target_binaries(void)
 {
-       int i;
-
-       __target_binaries_lock();
+       struct bin_desc *p, *n;
 
-       for (i = 0; i < target_binaries_cnt; i++) {
-               put_dentry(target_binaries[i].dentry);
-               kfree(target_binaries[i].filename);
+       down_write(&target_binaries_lock);
+       list_for_each_entry_safe(p, n, &target_binaries_list, list) {
+               list_del(&p->list);
+               __free_target_binary(p);
        }
-
-       kfree(target_binaries);
        target_binaries_cnt = 0;
-       target_binaries_slots = 0;
-
-       __target_binaries_unlock();
-}
-
-static int __grow_target_binaries(void)
-{
-       struct bin_desc *tmp = target_binaries;
-       int i, ret;
-
-       __target_binaries_lock();
-
-       ret = __alloc_target_binaries_no_lock(target_binaries_slots + DEFAULT_SLOTS_STEP);
-       if (ret != 0)
-               return ret;
-
-       for (i = 0; i < target_binaries_cnt; i++) {
-               target_binaries[i].dentry = tmp[i].dentry;
-               target_binaries[i].filename = tmp[i].filename;
-       }
-
-       __target_binaries_unlock();
-
-       kfree(tmp);
-
-       return 0;
+       up_write(&target_binaries_lock);
 }
 
 static bool __check_dentry_already_exist(struct dentry *dentry)
 {
-       int i;
+       struct bin_desc *p;
        bool ret = false;
 
-       __target_binaries_lock();
-
-       for (i = 0; i < target_binaries_cnt; i++) {
-               if (target_binaries[i].dentry == dentry) {
+       down_read(&target_binaries_lock);
+       list_for_each_entry(p, &target_binaries_list, list) {
+               if (p->dentry == dentry) {
                        ret = true;
-                       goto check_dentry_unlock;
+                       goto out;
                }
        }
+out:
+       up_read(&target_binaries_lock);
 
-check_dentry_unlock:
-       __target_binaries_unlock();
-
-       return ret;
+       return false;
 }
 
 static int __add_target_binary(struct dentry *dentry, char *filename)
 {
-       int ret;
+       struct bin_desc *p;
        size_t len;
 
        if (__check_dentry_already_exist(dentry)) {
@@ -135,35 +97,23 @@ static int __add_target_binary(struct dentry *dentry, char *filename)
                return EALREADY;
        }
 
-
-       if (target_binaries_slots == target_binaries_cnt) {
-               ret = __grow_target_binaries();
-               if (ret != 0)
-                       return ret;
-       }
-
        /* Filename should be < PATH_MAX */
        len = strnlen(filename, PATH_MAX);
        if (len == PATH_MAX)
                return -EINVAL;
 
-       __target_binaries_lock();
-
-       target_binaries[target_binaries_cnt].dentry = dentry;
-       target_binaries[target_binaries_cnt].filename = kmalloc(len + 1, GFP_KERNEL);
-       memcpy(target_binaries[target_binaries_cnt].filename, filename, len + 1);
-       ++target_binaries_cnt;
+       p = __alloc_target_binary(dentry, filename, len);
+       if (!p)
+               return -ENOMEM;
 
-       __target_binaries_unlock();
+       down_write(&target_binaries_lock);
+       list_add_tail(&p->list, &target_binaries_list);
+       target_binaries_cnt++;
+       up_write(&target_binaries_lock);
 
        return 0;
 }
 
-static char *__get_binary_name(struct bin_desc *bin)
-{
-       return bin->filename;
-}
-
 static struct dentry *__get_caller_dentry(struct task_struct *task,
                                          unsigned long caller)
 {
@@ -186,13 +136,7 @@ get_caller_dentry_fail:
 static bool __check_if_instrumented(struct task_struct *task,
                                    struct dentry *dentry)
 {
-       int i;
-
-       for (i = 0; i < target_binaries_cnt; i++)
-               if (target_binaries[i].dentry == dentry)
-                       return true;
-
-       return false;
+       return __check_dentry_already_exist(dentry);
 }
 
 static bool __is_instrumented(void *caller)
@@ -247,32 +191,35 @@ int preload_control_add_instrumented_binary(char *filename)
 int preload_control_clean_instrumented_bins(void)
 {
        __free_target_binaries();
-       return __alloc_target_binaries(DEFAULT_SLOTS_COUNT);
+
+       return 0;
 }
 
 unsigned int preload_control_get_bin_names(char ***filenames_p)
 {
-       int i;
-       unsigned int ret = 0;
+       unsigned int i, ret = 0;
+       struct bin_desc *p;
+       char **a = NULL;
 
+       down_read(&target_binaries_lock);
        if (target_binaries_cnt == 0)
-               return 0;
-
-       __target_binaries_lock();
-
-       *filenames_p = kmalloc(sizeof(**filenames_p) * target_binaries_cnt,
-                          GFP_KERNEL);
-       if (*filenames_p == NULL)
-               goto get_binaries_names_out;
+               goto out;
 
-       for (i = 0; i < target_binaries_cnt; i++)
-               (*filenames_p)[i] = __get_binary_name(&target_binaries[i]);
+       a = kmalloc(sizeof(*a) * target_binaries_cnt, GFP_KERNEL);
+       if (!a)
+               goto out;
 
-       ret = target_binaries_cnt;
-
-get_binaries_names_out:
-       __target_binaries_unlock();
+       i = 0;
+       list_for_each_entry(p, &target_binaries_list, list) {
+               if (i >= target_binaries_cnt)
+                       break;
+               a[i++] = p->filename;
+       }
 
+       *filenames_p = a;
+       ret = i;
+out:
+       up_read(&target_binaries_lock);
        return ret;
 }
 
@@ -283,7 +230,7 @@ void preload_control_release_bin_names(char ***filenames_p)
 
 int preload_control_init(void)
 {
-       return __alloc_target_binaries(DEFAULT_SLOTS_COUNT);
+       return 0;
 }
 
 void preload_control_exit(void)
@@ -291,5 +238,3 @@ void preload_control_exit(void)
        __free_target_binaries();
 }
 
-#undef DEFAULT_SLOTS_STEP
-#undef DEFAULT_SLOTS_COUNT