On 11/09/2011 11:12 AM, Dhaval Giani wrote:
> 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.gi...@gmail.com>
Acked-By: Jan Safranek <jsafr...@redhat.com>
> ---
>
> Changes from v1,
> 1. Correct return values for test case
> 2. Add a make check entry
>
> Hopefully Jan can give it a run and confirm stuff works as expected!
> Thanks!
>
> ---
> src/wrapper.c | 124 ++++++++++++-------------------------------------
> tests/.gitignore | 1 +
> tests/Makefile.am | 5 +-
> tests/wrapper_test.c | 44 ++++++++++++++++++
> 4 files changed, 79 insertions(+), 95 deletions(-)
> create mode 100644 tests/wrapper_test.c
>
> diff --git a/src/wrapper.c b/src/wrapper.c
> index 90c8bc3..95be969 100644
> --- 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 b6aeef7..803c49a 100644
> --- 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 4cdcbf3..0b77b42 100644
> --- 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
> 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;
> +}
------------------------------------------------------------------------------
RSA(R) Conference 2012
Save $700 by Nov 18
Register now
http://p.sf.net/sfu/rsa-sfdev2dev1
_______________________________________________
Libcg-devel mailing list
Libcg-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libcg-devel
