libcgroup wrappers: Remove duplicated code

There is a lot of duplicated code in the wrappers. This is a hinderance
when one wants to change the basic design and in general maintainence.

Remove the duplicated code.

Also add a test case to test the changes.

Signed-off-by: Dhaval Giani <dhaval.giani@gmail.com>
Acked-By: Jan Safranek <jsafrane@redhat.com>

diff --git a/src/wrapper.c b/src/wrapper.c
index 90c8bc31a0b2995f1327042bfa06b2f8da235daf..95be969678708df33f179134f25971c57e6a3c65 100644 (file)
--- a/src/wrapper.c
+++ b/src/wrapper.c
@@ -15,6 +15,9 @@
  * his mistake.
  */
 
+#define _GNU_SOURCE
+
+#include <errno.h>
 #include <libcgroup.h>
 #include <libcgroup-internal.h>
 #include <inttypes.h>
@@ -150,123 +153,58 @@ int cgroup_add_value_string(struct cgroup_controller *controller,
 int cgroup_add_value_int64(struct cgroup_controller *controller,
                                        const char *name, int64_t value)
 {
-       int i;
-       unsigned ret;
-       struct control_value *cntl_value;
-
-       if (!controller)
-               return ECGINVAL;
-
-       if (controller->index >= CG_VALUE_MAX)
-               return ECGMAXVALUESEXCEEDED;
-
-       for (i = 0; i < controller->index && i < CG_VALUE_MAX; i++) {
-               if (!strcmp(controller->values[i]->name, name))
-                       return ECGVALUEEXISTS;
-       }
-
-       cntl_value = calloc(1, sizeof(struct control_value));
-
-       if (!cntl_value)
-               return ECGCONTROLLERCREATEFAILED;
-
-       strncpy(cntl_value->name, name,
-                       sizeof(cntl_value->name));
-       ret = snprintf(cntl_value->value,
-         sizeof(cntl_value->value), "%" PRId64, value);
+       int ret;
+       char *val;
 
-       if (ret >= sizeof(cntl_value->value)) {
-               free(cntl_value);
-               return ECGINVAL;
+       ret = asprintf(&val, "%"PRId64, value);
+       if (ret < 0) {
+               last_errno = errno;
+               return ECGOTHER;
        }
 
-       controller->values[controller->index] = cntl_value;
-       controller->index++;
-
-       return 0;
+       ret = cgroup_add_value_string(controller, name, val);
+       free(val);
 
+       return ret;
 }
 
 int cgroup_add_value_uint64(struct cgroup_controller *controller,
                                        const char *name, u_int64_t value)
 {
-       int i;
-       unsigned ret;
-       struct control_value *cntl_value;
-
-       if (!controller)
-               return ECGINVAL;
-
-       if (controller->index >= CG_VALUE_MAX)
-               return ECGMAXVALUESEXCEEDED;
-
-       for (i = 0; i < controller->index && i < CG_VALUE_MAX; i++) {
-               if (!strcmp(controller->values[i]->name, name))
-                       return ECGVALUEEXISTS;
-       }
-
-       cntl_value = calloc(1, sizeof(struct control_value));
-
-       if (!cntl_value)
-               return ECGCONTROLLERCREATEFAILED;
-
-       strncpy(cntl_value->name, name, sizeof(cntl_value->name));
-       ret = snprintf(cntl_value->value, sizeof(cntl_value->value),
-                                               "%" PRIu64, value);
+       int ret;
+       char *val;
 
-       if (ret >= sizeof(cntl_value->value)) {
-               free(cntl_value);
-               return ECGINVAL;
+       ret = asprintf(&val, "%" PRIu64, value);
+       if (ret < 0) {
+               last_errno = errno;
+               return ECGOTHER;
        }
 
-       controller->values[controller->index] = cntl_value;
-       controller->index++;
-
-       return 0;
+       ret = cgroup_add_value_string(controller, name, val);
+       free(val);
 
+       return ret;
 }
 
 int cgroup_add_value_bool(struct cgroup_controller *controller,
                                                const char *name, bool value)
 {
-       int i;
-       unsigned ret;
-       struct control_value *cntl_value;
-
-       if (!controller)
-               return ECGINVAL;
-
-       if (controller->index >= CG_VALUE_MAX)
-               return ECGMAXVALUESEXCEEDED;
-
-       for (i = 0; i < controller->index && i < CG_VALUE_MAX; i++) {
-               if (!strcmp(controller->values[i]->name, name))
-                       return ECGVALUEEXISTS;
-       }
-
-       cntl_value = calloc(1, sizeof(struct control_value));
-
-       if (!cntl_value)
-               return ECGCONTROLLERCREATEFAILED;
-
-       strncpy(cntl_value->name, name, sizeof(cntl_value->name));
+       int ret;
+       char *val;
 
        if (value)
-               ret = snprintf(cntl_value->value,
-                               sizeof(cntl_value->value), "1");
+               val = strdup("1");
        else
-               ret = snprintf(cntl_value->value,
-                               sizeof(cntl_value->value), "0");
-
-       if (ret >= sizeof(cntl_value->value)) {
-               free(cntl_value);
-               return ECGINVAL;
+               val = strdup("0");
+       if (!val) {
+               last_errno = errno;
+               return ECGOTHER;
        }
 
-       controller->values[controller->index] = cntl_value;
-       controller->index++;
+       ret = cgroup_add_value_string(controller, name, val);
+       free(val);
 
-       return 0;
+       return ret;
 }
 
 int cgroup_compare_controllers(struct cgroup_controller *cgca,

diff --git a/tests/.gitignore b/tests/.gitignore
index b6aeef7408927b1a96f1f5af1f7b0c856f02edea..803c49ac8e2d8c9e47807d3382b41445c9de450e 100644 (file)
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -12,3 +12,4 @@ setuid
 test_named_hierarchy
 walk_task
 walk_test
+wrapper_test

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 4cdcbf366a207065d83096934b2d83dc67710659..0b77b42bb176a5783039c14023c81f3cb90ec0b9 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -4,7 +4,7 @@ INCLUDES = -I$(top_srcdir)/include
 LDADD = $(top_builddir)/src/.libs/libcgroup.la
 
 # compile the tests, but do not install them
-noinst_PROGRAMS = libcgrouptest01 libcg_ba setuid walk_test read_stats walk_task get_controller get_mount_point proctest get_all_controller get_variable_names test_named_hierarchy get_procs
+noinst_PROGRAMS = libcgrouptest01 libcg_ba setuid walk_test read_stats walk_task get_controller get_mount_point proctest get_all_controller get_variable_names test_named_hierarchy get_procs wrapper_test
 
 libcgrouptest01_SOURCES=libcgrouptest01.c test_functions.c libcgrouptest.h
 libcg_ba_SOURCES=libcg_ba.cpp
@@ -19,7 +19,8 @@ get_all_controller_SOURCES=get_all_controller.c
 get_variable_names_SOURCES=get_variable_names.c
 test_named_hierarchy_SOURCES=test_named_hierarchy.c
 get_procs_SOURCES=get_procs.c
+wrapper_test_SOURCES=wrapper_test.c
 
 EXTRA_DIST = runlibcgrouptest.sh
 
-TESTS = runlibcgrouptest.sh
+TESTS = wrapper_test runlibcgrouptest.sh

diff --git a/tests/wrapper_test.c b/tests/wrapper_test.c
new file mode 100644 (file)
index 0000000..257ece3
--- /dev/null
+++ b/tests/wrapper_test.c
@@ -0,0 +1,44 @@
+#include <libcgroup.h>
+#include <stdio.h>
+#include <string.h>
+#include "../src/libcgroup-internal.h"
+
+int main()
+{
+       struct cgroup *cgroup;
+       struct cgroup_controller *cgc;
+       int fail = 0;
+
+       cgroup = cgroup_new_cgroup("test");
+       cgc = cgroup_add_controller(cgroup, "cpu");
+
+       cgroup_add_value_int64(cgc, "cpu.shares", 2048);
+       cgroup_add_value_uint64(cgc, "cpu.something", 1000);
+       cgroup_add_value_bool(cgc, "cpu.bool", 1);
+
+       if (!strcmp(cgroup->controller[0]->values[0]->name, "cpu.shares")) {
+               if (strcmp(cgroup->controller[0]->values[0]->value, "2048")) {
+                       printf("FAIL for add_value_int\n");
+                       fail = 1;
+               }
+       }
+
+       if (!strcmp(cgroup->controller[0]->values[1]->name, "cpu.something")) {
+               if (strcmp(cgroup->controller[0]->values[1]->value, "1000")) {
+                       printf("FAIL for add_value_uint\n");
+                       fail = 1;
+               }
+       }
+
+       if (!strcmp(cgroup->controller[0]->values[2]->name, "cpu.bool")) {
+               if (strcmp(cgroup->controller[0]->values[2]->value, "1")) {
+                       printf("FAIL for add_value_bool\n");
+                       fail = 1;
+               }
+       }
+
+       if (!fail)
+               printf("PASS!\n");
+
+       return fail;
+}