A lack of proper initial thought let me use some variables globaly, which is not a good idea at all. However fixing this issue at this stage is not very good from the point that it changes variables for a lot of functions. But still it has to be done for a better and reusable code. This patch does that while putting related variables in meaningful structs. Thanks to Balbir for a good suggestion.
Signed-off-by: Sudhir Kumar <[email protected]> --- tests/libcgrouptest.h | 48 ++++++++++------- tests/libcgrouptest01.c | 131 ++++++++++++++++++++++++++---------------------- 2 files changed, 101 insertions(+), 78 deletions(-) Index: trunk/tests/libcgrouptest.h =================================================================== --- trunk.orig/tests/libcgrouptest.h +++ trunk/tests/libcgrouptest.h @@ -34,8 +34,6 @@ #define PASS 1 /* test passed */ #define FAIL 0 /* test failed */ -int cpu = 0, memory = 0; - enum cgroup_mount_t { FS_NOT_MOUNTED, FS_MOUNTED, @@ -78,26 +76,36 @@ enum info_message_t { NOMESSAGE, }; +/* Keep a single struct of all ids */ +struct uid_gid_t { + uid_t control_uid; + gid_t control_gid; + uid_t tasks_uid; + gid_t tasks_gid; +}; + +/* Keep a single struct of all control values */ +struct cntl_val_t { + int64_t val_int64; + u_int64_t val_uint64; + bool val_bool; + /* size worth of 100 digit num is fair enough */ + char val_string[100]; /* string value of control parameter */ +}; + +extern int cpu, memory; + /* The set of verbose messages useful to the user */ extern char info[NUM_MSGS][SIZE]; -int64_t val_int64; -u_int64_t val_uint64; -bool val_bool; -/* Doubt: size of following string. is'nt this wrong ?*/ -char val_string[FILENAME_MAX]; /* string value of control parameter */ -uid_t control_uid; -gid_t control_gid; -uid_t tasks_uid; -gid_t tasks_gid; /* this variable is never modified */ -int fs_mounted; +extern int fs_mounted; /* The mountpoints as received from script */ -char mountpoint[FILENAME_MAX], mountpoint2[FILENAME_MAX]; +extern char mountpoint[FILENAME_MAX], mountpoint2[FILENAME_MAX]; /* No extra message unless specified */ -char extra[SIZE] = "\n"; +extern char extra[SIZE]; /* Functions to test each API */ void test_cgroup_init(int retcode, int i); @@ -105,7 +113,7 @@ void test_cgroup_attach_task(int retcode const char *group1, const char *group2, int k, int i); struct cgroup *create_new_cgroup_ds(int ctl, const char *grpname, - int value_type, int i); + int value_type, struct cntl_val_t cval, struct uid_gid_t ids, int i); void test_cgroup_create_cgroup(int retcode, struct cgroup *cgrp, const char *name, int common, int mpnt, int ign, int i); void test_cgroup_delete_cgroup(int retcode, struct cgroup *cgrp, @@ -113,7 +121,7 @@ void test_cgroup_delete_cgroup(int retco void test_cgroup_modify_cgroup(int retcode, struct cgroup *cgrp, const char *name, int which_ctl, int ctl1, int ctl2, int value_type, int i); -void test_cgroup_get_cgroup(int ctl1, int ctl2, int i); +void test_cgroup_get_cgroup(int ctl1, int ctl2, struct uid_gid_t ids, int i); /* API test functions end here */ void test_cgroup_compare_cgroup(int ctl1, int ctl2, int i); @@ -122,11 +130,13 @@ void get_controllers(const char *name, i static int group_exist(char *path_group); static int set_controller(int controller, char *controller_name, char *control_file); -static int group_modified(char *path_control_file, int value_type); +static int group_modified(char *path_control_file, int value_type, + struct cntl_val_t cval); static int add_control_value(struct cgroup_controller *newcontroller, - char * control_file, char *wr, int value_type); + char *control_file, char *wr, int value_type, struct cntl_val_t cval); struct cgroup *new_cgroup(char *group, char *controller_name, - char *control_file, int value_type, int i); + char *control_file, int value_type, struct cntl_val_t cval, + struct uid_gid_t ids, int i); int check_fsmounted(int multimnt); static int check_task(char *tasksfile); /* function to print messages in better format */ Index: trunk/tests/libcgrouptest01.c =================================================================== --- trunk.orig/tests/libcgrouptest01.c +++ trunk/tests/libcgrouptest01.c @@ -40,9 +40,17 @@ char info[NUM_MSGS][SIZE] = { "\n", /* NOMESSAGE */ }; + +int cpu, memory; +int fs_mounted; +char mountpoint[FILENAME_MAX], mountpoint2[FILENAME_MAX]; +char extra[SIZE] = "\n"; + int main(int argc, char *argv[]) { int retval; + struct uid_gid_t ids = {0}; /* Set default control permissions */ + struct cntl_val_t cval = {2000, 2000, 1, "2000"}; struct cgroup *cgroup1, *cgroup2, *cgroup3, *nullcgroup = NULL; struct cgroup_controller *sec_controller; /* In case of multimount for readability we use the controller name @@ -90,12 +98,6 @@ int main(int argc, char *argv[]) exit(1); } - /* Set default control permissions */ - control_uid = 0; - control_gid = 0; - tasks_uid = 0; - tasks_gid = 0; - /* * Testsets: Testcases are broadly devided into 3 categories based on * filesystem(fs) mount scenario. fs not mounted, fs mounted, fs multi @@ -126,7 +128,8 @@ int main(int argc, char *argv[]) * Exp outcome: no error */ - cgroup1 = create_new_cgroup_ds(0, "group1", STRING, 3); + cgroup1 = create_new_cgroup_ds(0, "group1", + STRING, cval, ids, 3); /* * Test04: Then Call cgroup_create_cgroup() with this valid grp @@ -212,12 +215,13 @@ int main(int argc, char *argv[]) * Test05: Create a valid cgroup structure * Exp outcome: no error. 0 return value */ - cgroup1 = create_new_cgroup_ds(ctl1, "group1", STRING, 5); + cgroup1 = create_new_cgroup_ds(ctl1, "group1", + STRING, cval, ids, 5); if (!cgroup1) { fprintf(stderr, "Failed to create new cgroup ds. " "Trying with second controller\n"); cgroup1 = create_new_cgroup_ds(ctl2, "group1", STRING, - 5); + cval, ids, 5); if (!cgroup1) { fprintf(stderr, "Failed to create cgroup ds. " "Tests dependent on this structure " @@ -249,8 +253,8 @@ int main(int argc, char *argv[]) build_path(path_control_file, mountpoint, "group1", control_file); retval = cgroup_modify_cgroup(cgroup1); - /* Check if the values are changed */ - if (!retval && !group_modified(path_control_file, STRING)) + /* Check if the values are changed. cval contains orig values */ + if (!retval && !group_modified(path_control_file, STRING, cval)) message(8, PASS, "modify_cgroup()", retval, info[SAMEGRP]); else @@ -261,12 +265,13 @@ int main(int argc, char *argv[]) * Create another valid cgroup structure with same group * to modify the existing group */ - cgroup2 = create_new_cgroup_ds(ctl1, "group1", STRING, 9); + cgroup2 = create_new_cgroup_ds(ctl1, "group1", + STRING, cval, ids, 9); if (!cgroup2) { fprintf(stderr, "Failed to create new cgroup ds. " "Trying with second controller\n"); - cgroup2 = create_new_cgroup_ds(ctl2, "group1", STRING, - 9); + cgroup2 = create_new_cgroup_ds(ctl2, "group1", + STRING, cval, ids, 9); if (!cgroup2) { fprintf(stderr, "Failed to create cgroup ds. " "Tests dependent on this structure " @@ -296,8 +301,9 @@ int main(int argc, char *argv[]) * Create another valid cgroup structure with diff controller * to modify the existing group */ - val_int64 = 262144; - cgroup3 = create_new_cgroup_ds(ctl2, "group1", INT64, 12); + cval.val_int64 = 262144; + cgroup3 = create_new_cgroup_ds(ctl2, "group1", + INT64, cval, ids, 12); if (!cgroup3) { fprintf(stderr, "Failed to create new cgroup ds. " "Tests dependent on this structure " @@ -321,7 +327,7 @@ int main(int argc, char *argv[]) * that "group1" exists in fs. So call cgroup_create_cgroup() * with "group1" named group before calling this test function. */ - test_cgroup_get_cgroup(ctl1, ctl2, 14); + test_cgroup_get_cgroup(ctl1, ctl2, ids, 14); /* * Test16: delete cgroup @@ -391,7 +397,7 @@ int main(int argc, char *argv[]) * Exp outcome: no error. 0 return value */ ctl1_cgroup1 = create_new_cgroup_ds(ctl1, "ctl1_group1", - STRING, 3); + STRING, cval, ids, 3); if (!ctl1_cgroup1) { fprintf(stderr, "Failed to create new cgroup ds. " "Tests dependent on this structure " @@ -411,7 +417,7 @@ int main(int argc, char *argv[]) * Exp outcome: no error. 0 return value */ ctl2_cgroup1 = create_new_cgroup_ds(ctl2, "ctl2_group1", - STRING, 5); + STRING, cval, ids, 5); if (!ctl2_cgroup1) { fprintf(stderr, "Failed to create new cgroup ds. " "Tests dependent on this structure " @@ -459,7 +465,7 @@ int main(int argc, char *argv[]) * Exp outcome: no error. 0 return value */ ctl2_cgroup2 = create_new_cgroup_ds(ctl2, "ctl2_group2", - STRING, 10); + STRING, cval, ids, 10); if (!ctl2_cgroup2) { fprintf(stderr, "Failed to create new cgroup ds. " "Tests dependent on this structure " @@ -481,7 +487,7 @@ int main(int argc, char *argv[]) * Exp outcome: no error. 0 return value */ mod_ctl1_cgroup1 = create_new_cgroup_ds(ctl1, "ctl1_group1", - STRING, 12); + STRING, cval, ids, 12); if (!mod_ctl1_cgroup1) { fprintf(stderr, "Failed to create new cgroup ds. " "Tests dependent on this structure " @@ -502,7 +508,7 @@ int main(int argc, char *argv[]) * Exp outcome: no error. 0 return value */ mod_ctl2_cgroup1 = create_new_cgroup_ds(ctl2, "ctl2_group1", - STRING, 14); + STRING, cval, ids, 14); if (!mod_ctl2_cgroup1) { fprintf(stderr, "Failed to create new cgroup ds. " "Tests dependent on this structure " @@ -537,7 +543,7 @@ int main(int argc, char *argv[]) * Exp outcome: no error. 0 return value */ common_cgroup = create_new_cgroup_ds(ctl1, "commongroup", - STRING, 18); + STRING, cval, ids, 18); if (!common_cgroup) { fprintf(stderr, "Failed to create new cgroup ds. " "Tests dependent on this structure " @@ -582,7 +588,7 @@ int main(int argc, char *argv[]) * Exp outcome: no error. 0 return value */ mod_common_cgroup = create_new_cgroup_ds(ctl1, "commongroup", - STRING, 21); + STRING, cval, ids, 21); if (!common_cgroup) { fprintf(stderr, "Failed to create new cgroup ds. " "Tests dependent on this structure " @@ -607,9 +613,9 @@ int main(int argc, char *argv[]) exit(1); } - strncpy(val_string, "7000064", sizeof(val_string)); + strncpy(cval.val_string, "7000064", sizeof(cval.val_string)); retval = cgroup_add_value_string(sec_controller, - control_file, val_string); + control_file, cval.val_string); if (retval) printf("The cgroup_modify_cgroup() test will fail\n"); @@ -626,7 +632,7 @@ int main(int argc, char *argv[]) */ test_cgroup_delete_cgroup(0, common_cgroup, "commongroup", 1, 2, 1, 23); - test_cgroup_get_cgroup(ctl1, ctl2, 24); + test_cgroup_get_cgroup(ctl1, ctl2, ids, 24); /* Free the cgroup structures */ cgroup_free(&nullcgroup); @@ -707,7 +713,7 @@ void test_cgroup_attach_task(int retcode struct cgroup *create_new_cgroup_ds(int ctl, const char *grpname, - int value_type, int i) + int value_type, struct cntl_val_t cval, struct uid_gid_t ids, int i) { int retval; char group[FILENAME_MAX]; @@ -720,16 +726,15 @@ struct cgroup *create_new_cgroup_ds(int return NULL; } - /* val_string is still global. Will replace soon with config file */ switch (ctl) { /* control values are controller specific, so will be set * accordingly from the config file */ case CPU: - strncpy(val_string, "260000", sizeof(val_string)); + strncpy(cval.val_string, "260000", sizeof(cval.val_string)); break; case MEMORY: - strncpy(val_string, "7000064", sizeof(val_string)); + strncpy(cval.val_string, "7000064", sizeof(cval.val_string)); break; /* To be added for other controllers */ @@ -740,7 +745,8 @@ struct cgroup *create_new_cgroup_ds(int break; } - return new_cgroup(group, controller_name, control_file, value_type, i); + return new_cgroup(group, controller_name, control_file, + value_type, cval, ids, i); } @@ -868,6 +874,7 @@ void test_cgroup_modify_cgroup(int retco int ctl2, int value_type, int i) { int retval; + struct cntl_val_t cval; char path1_control_file[FILENAME_MAX], path2_control_file[FILENAME_MAX]; char controller_name[FILENAME_MAX], control_file[FILENAME_MAX]; @@ -899,9 +906,9 @@ void test_cgroup_modify_cgroup(int retco set_controller(ctl1, controller_name, control_file); build_path(path1_control_file, mountpoint, name, control_file); /* this approach will be changed in coming patches */ - strncpy(val_string, "260000", sizeof(val_string)); + strncpy(cval.val_string, "260000", sizeof(cval.val_string)); - if (!group_modified(path1_control_file, value_type)) + if (!group_modified(path1_control_file, value_type, cval)) message(i, PASS, "modify_cgroup()", retval, info[NOMESSAGE]); else @@ -921,8 +928,8 @@ void test_cgroup_modify_cgroup(int retco name, control_file); /* this approach will be changed in coming patches */ - strncpy(val_string, "7000064", sizeof(val_string)); - if (!group_modified(path2_control_file, value_type)) + strncpy(cval.val_string, "7000064", sizeof(cval.val_string)); + if (!group_modified(path2_control_file, value_type, cval)) message(i, PASS, "modify_cgroup()", retval, info[NOMESSAGE]); else @@ -947,10 +954,12 @@ void test_cgroup_modify_cgroup(int retco name, control_file); } /* this approach will be changed in coming patches */ - strncpy(val_string, "260000", sizeof(val_string)); - if (!group_modified(path1_control_file, value_type)) { - strncpy(val_string, "7000064", sizeof(val_string)); - if (!group_modified(path2_control_file, value_type)) + strncpy(cval.val_string, "260000", sizeof(cval.val_string)); + if (!group_modified(path1_control_file, value_type, cval)) { + strncpy(cval.val_string, "7000064", + sizeof(cval.val_string)); + if (!group_modified(path2_control_file, + value_type, cval)) message(i, PASS, "modify_cgroup()", retval, info[GRPMODINBOTHCTLS]); else @@ -1035,7 +1044,8 @@ static int set_controller(int controller } } -static int group_modified(char *path_control_file, int value_type) +static int group_modified(char *path_control_file, int value_type, + struct cntl_val_t cval) { bool bool_val; int64_t int64_val; @@ -1054,22 +1064,22 @@ static int group_modified(char *path_con case BOOL: fscanf(fd, "%d", &bool_val); - if (bool_val == val_bool) + if (bool_val == cval.val_bool) return 0; break; case INT64: fscanf(fd, "%lld", &int64_val); - if (int64_val == val_int64) + if (int64_val == cval.val_int64) return 0; break; case UINT64: fscanf(fd, "%llu", &uint64_val); - if (uint64_val == val_uint64) + if (uint64_val == cval.val_uint64) return 0; break; case STRING: fscanf(fd, "%s", string_val); - if (!strncmp(string_val, val_string, strlen(string_val))) + if (!strncmp(string_val, cval.val_string, strlen(string_val))) return 0; break; default: @@ -1082,7 +1092,7 @@ static int group_modified(char *path_con return 1; } static int add_control_value(struct cgroup_controller *newcontroller, - char * control_file, char *wr, int value_type) + char *control_file, char *wr, int value_type, struct cntl_val_t cval) { int retval; @@ -1090,22 +1100,22 @@ static int add_control_value(struct cgro case BOOL: retval = cgroup_add_value_bool(newcontroller, - control_file, val_bool); + control_file, cval.val_bool); snprintf(wr, SIZE, "add_value_bool()"); break; case INT64: retval = cgroup_add_value_int64(newcontroller, - control_file, val_int64); + control_file, cval.val_int64); snprintf(wr, SIZE, "add_value_int64()"); break; case UINT64: retval = cgroup_add_value_uint64(newcontroller, - control_file, val_uint64); + control_file, cval.val_uint64); snprintf(wr, SIZE, "add_value_uint64()"); break; case STRING: retval = cgroup_add_value_string(newcontroller, - control_file, val_string); + control_file, cval.val_string); snprintf(wr, SIZE, "add_value_string()"); break; default: @@ -1117,7 +1127,8 @@ static int add_control_value(struct cgro } struct cgroup *new_cgroup(char *group, char *controller_name, - char *control_file, int value_type, int i) + char *control_file, int value_type, + struct cntl_val_t cval, struct uid_gid_t ids, int i) { int retval; char wr[SIZE]; /* Names of wrapper apis */ @@ -1127,8 +1138,8 @@ struct cgroup *new_cgroup(char *group, c newcgroup = cgroup_new_cgroup(group); if (newcgroup) { - retval = cgroup_set_uid_gid(newcgroup, tasks_uid, tasks_gid, - control_uid, control_gid); + retval = cgroup_set_uid_gid(newcgroup, ids.tasks_uid, + ids.tasks_gid, ids.control_uid, ids.control_gid); if (retval) { snprintf(wr, SIZE, "set_uid_gid()"); @@ -1139,7 +1150,7 @@ struct cgroup *new_cgroup(char *group, c controller_name); if (newcontroller) { retval = add_control_value(newcontroller, - control_file, wr, value_type); + control_file, wr, value_type, cval); if (!retval) { message(i++, PASS, "new_cgroup()", @@ -1259,6 +1270,7 @@ static inline void build_path(char *targ void test_cgroup_compare_cgroup(int ctl1, int ctl2, int i) { int retval; + struct cntl_val_t cval = {0, 0, 0, "5000"}; struct cgroup *cgroup1, *cgroup2; struct cgroup_controller *controller; char controller_name[FILENAME_MAX], control_file[FILENAME_MAX]; @@ -1280,7 +1292,7 @@ void test_cgroup_compare_cgroup(int ctl1 controller = cgroup_add_controller(cgroup1, controller_name); if (controller) { retval = add_control_value(controller, - control_file, wr, STRING); + control_file, wr, STRING, cval); if (retval) message(i++, FAIL, wr, retval, extra); } @@ -1288,7 +1300,7 @@ void test_cgroup_compare_cgroup(int ctl1 controller = cgroup_add_controller(cgroup2, controller_name); if (controller) { retval = add_control_value(controller, - control_file, wr, STRING); + control_file, wr, STRING, cval); if (retval) message(i++, FAIL, wr, retval, extra); } @@ -1304,7 +1316,7 @@ void test_cgroup_compare_cgroup(int ctl1 controller = cgroup_add_controller(cgroup2, controller_name); if (controller) { retval = add_control_value(controller, - control_file, wr, STRING); + control_file, wr, STRING, cval); if (retval) message(i++, FAIL, wr, retval, extra); } @@ -1319,11 +1331,12 @@ void test_cgroup_compare_cgroup(int ctl1 cgroup_free(&cgroup2); } -void test_cgroup_get_cgroup(int ctl1, int ctl2, int i) +void test_cgroup_get_cgroup(int ctl1, int ctl2, struct uid_gid_t ids, int i) { struct cgroup *cgroup_filled, *cgroup_a, *cgroup_b; struct cgroup_controller *controller; char controller_name[FILENAME_MAX], control_file[FILENAME_MAX]; + struct cntl_val_t cval = {0, 0, 0, "5000"}; int ret; /* @@ -1371,7 +1384,7 @@ void test_cgroup_get_cgroup(int ctl1, in /* MULTIMOUNT: Create, get and compare a cgroup under both mounts */ /* get cgroup_a ds and create group_a in filesystem */ - cgroup_a = create_new_cgroup_ds(ctl1, "group_a", STRING, 00); + cgroup_a = create_new_cgroup_ds(ctl1, "group_a", STRING, cval, ids, 0); if (fs_mounted == FS_MULTI_MOUNTED) { /* Create under another controller also */ ret = set_controller(ctl2, controller_name, control_file); ------------------------------------------------------------------------------ _______________________________________________ Libcg-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/libcg-devel
