From e8e46279a22b7cbc0af1d916264efc7d9dc96086 Mon Sep 17 00:00:00 2001 From: Dmitri Pal Date: Fri, 20 Jul 2012 13:34:43 -0400 Subject: [PATCH 01/10] [INI] Fix permission checking unit test The unit test was broken. The wrong function was used. To make sure everything is correct I also added a convenience function to print the internals if the file context object. The unit test is fixed to use relative paths consitently. Also added nice statements at the beggining and the end of the unit test functions where they were missing. --- ini/ini_fileobj.c | 41 ++++++++++++++- ini/ini_parse_ut.c | 154 +++++++++++++++++++++++++++++++++++++++------------- 2 files changed, 157 insertions(+), 38 deletions(-) diff --git a/ini/ini_fileobj.c b/ini/ini_fileobj.c index 3f348a7..8193bc2 100644 --- a/ini/ini_fileobj.c +++ b/ini/ini_fileobj.c @@ -485,7 +485,7 @@ int ini_config_access_check(struct ini_cfgfile *file_ctx, } -/* Determins if two file contexts are different by comparing: +/* Determines if two file contexts are different by comparing: * - time stamp * - device ID * - i-node @@ -505,6 +505,13 @@ int ini_config_changed(struct ini_cfgfile *file_ctx1, *changed = 0; + /* Unfortunately the time is not granular enough + * to detect the change if run during the unit test. + * In future when a more granular version of stat + * is available we should switch to it and update + * the unit test. + */ + if((file_ctx1->file_stats.st_mtime != file_ctx2->file_stats.st_mtime) || (file_ctx1->file_stats.st_dev != @@ -518,3 +525,35 @@ int ini_config_changed(struct ini_cfgfile *file_ctx1, TRACE_FLOW_EXIT(); return EOK; } + +/* Print the file object contents */ +void ini_config_file_print(struct ini_cfgfile *file_ctx) +{ + TRACE_FLOW_ENTRY(); + if (file_ctx == NULL) { + printf("No file object\n."); + } + else { + printf("File name: %s\n", (file_ctx->filename) ? file_ctx->filename : "NULL"); + printf("File is %s\n", (file_ctx->file) ? "open" : "closed"); + printf("Error level is %d\n", file_ctx->error_level); + printf("Collision flags %u\n", file_ctx->collision_flags); + printf("Metadata flags %u\n", file_ctx->metadata_flags); + if (file_ctx->error_list) col_print_collection(file_ctx->error_list); + else printf("Error list is empty."); + printf("Stats flag st_dev %li\n", file_ctx->file_stats.st_dev); + printf("Stats flag st_ino %li\n", file_ctx->file_stats.st_ino); + printf("Stats flag st_mode %u\n", file_ctx->file_stats.st_mode); + printf("Stats flag st_nlink %li\n", file_ctx->file_stats.st_nlink); + printf("Stats flag st_uid %u\n", file_ctx->file_stats.st_uid); + printf("Stats flag st_gid %u\n", file_ctx->file_stats.st_gid); + printf("Stats flag st_rdev %li\n", file_ctx->file_stats.st_rdev); + printf("Stats flag st_size %lu\n", file_ctx->file_stats.st_size); + printf("Stats flag st_blocks %li\n", file_ctx->file_stats.st_blocks); + printf("Stats flag st_atime %ld\n", file_ctx->file_stats.st_atime); + printf("Stats flag st_mtime %ld\n", file_ctx->file_stats.st_mtime); + printf("Stats flag st_ctime %ld\n", file_ctx->file_stats.st_ctime); + printf("Count %u\n", file_ctx->count); + } + TRACE_FLOW_EXIT(); +} diff --git a/ini/ini_parse_ut.c b/ini/ini_parse_ut.c index 826cce0..c3ee5fd 100644 --- a/ini/ini_parse_ut.c +++ b/ini/ini_parse_ut.c @@ -42,8 +42,6 @@ char *confdir = NULL; if (verbose) foo; \ } while(0) -#define FOO_CONF "./ini/ini.d/foo.conf" - typedef int (*test_fn)(void); int test_one_file(const char *in_filename, @@ -73,7 +71,7 @@ int test_one_file(const char *in_filename, 0, /* TBD */ &file_ctx); if (error) { - printf("Failed to open file for reading. Error %d.\n", error); + printf("Failed to open file for %s reading. Error %d.\n", in_filename, error); ini_config_destroy(ini_config); return error; } @@ -170,7 +168,8 @@ int read_save_test(void) int i = 0; char infile[PATH_MAX]; char outfile[PATH_MAX]; - char *srcdir; + char *srcdir = NULL; + char *builddir = NULL; const char *files[] = { "real", "mysssd", "ipa", @@ -178,6 +177,7 @@ int read_save_test(void) "smerge", NULL }; + INIOUT(printf("<==== Read save test ====>\n")); srcdir = getenv("srcdir"); @@ -185,12 +185,15 @@ int read_save_test(void) sprintf(infile, "%s/ini/ini.d/%s.conf", (srcdir == NULL) ? "." : srcdir, files[i]); - sprintf(outfile, "./%s.conf.out", files[i]); + sprintf(outfile, "%s/%s.conf.out", (builddir == NULL) ? "." : builddir, + files[i]); error = test_one_file(infile, outfile); INIOUT(printf("Test for file: %s returned %d\n", files[i], error)); i++; } + INIOUT(printf("<==== Read save test end ====>\n")); + return EOK; } @@ -202,6 +205,8 @@ int read_again_test(void) char infile[PATH_MAX]; char outfile[PATH_MAX]; char command[PATH_MAX * 3]; + char *srcdir = NULL; + char *builddir = NULL; const char *files[] = { "real", "mysssd", "ipa", @@ -209,11 +214,16 @@ int read_again_test(void) "smerge", NULL }; + INIOUT(printf("<==== Read again test ====>\n")); + + srcdir = getenv("srcdir"); while(files[i]) { - sprintf(infile, "./%s.conf.out", files[i]); - sprintf(outfile, "./%s.conf.2.out", files[i]); + sprintf(infile, "%s/%s.conf.out", + (builddir == NULL) ? "." : builddir, files[i]); + sprintf(outfile, "%s/%s.conf.2.out", + (builddir == NULL) ? "." : builddir, files[i]); error = test_one_file(infile, outfile); INIOUT(printf("Test for file: %s returned %d\n", files[i], error)); if (error) break; @@ -226,6 +236,8 @@ int read_again_test(void) i++; } + INIOUT(printf("<==== Read again test end ====>\n")); + return error; } @@ -329,8 +341,14 @@ int merge_values_test(void) const char *resname = "./merge.conf.out"; const char *checkname = "./expect.conf.out"; char command[PATH_MAX * 3]; + char *srcdir = NULL; + + INIOUT(printf("<==== Merge values test ====>\n")); - memcpy(filename, FOO_CONF, sizeof(FOO_CONF)); + srcdir = getenv("srcdir"); + + sprintf(filename, "%s/ini/ini.d/foo.conf", + (srcdir == NULL) ? "." : srcdir); error = simplebuffer_alloc(&sbobj); if (error) { @@ -358,7 +376,8 @@ int merge_values_test(void) 0, /* TBD */ &file_ctx); if (error) { - printf("Failed to open file for reading. Error %d.\n", error); + printf("Failed to open %s file for reading. Error %d.\n", filename, error); + printf("Src is %s.\n", (srcdir == NULL) ? "NOT DEFINED" : srcdir); ini_config_destroy(ini_config); simplebuffer_free(sbobj); return error; @@ -439,6 +458,8 @@ int merge_values_test(void) INIOUT(printf("Comparison of %s %s returned: %d\n", resname, checkname, error)); + INIOUT(printf("<==== Merge values test end ====>\n")); + return error; } @@ -481,16 +502,23 @@ int merge_section_test(void) char filename[PATH_MAX]; char checkname[PATH_MAX]; - const char *resname = "./smerge.conf.out"; + char resname[PATH_MAX]; char command[PATH_MAX * 3]; char mode[100]; - char *srcdir; + char *srcdir = NULL; + char *builddir = NULL; + + + INIOUT(printf("<==== Merge section test ====>\n")); srcdir = getenv("srcdir"); + builddir = getenv("builddir"); sprintf(filename, "%s/ini/ini.d/smerge.conf", (srcdir == NULL) ? "." : srcdir); sprintf(checkname, "%s/ini/ini.d/sexpect.conf", (srcdir == NULL) ? "." : srcdir); + sprintf(resname, "%s/smerge.conf.out", + (builddir == NULL) ? "." : builddir); error = simplebuffer_alloc(&sbobj); if (error) { @@ -535,8 +563,9 @@ int merge_section_test(void) 0, /* TBD */ &file_ctx); if (error) { - printf("Failed to open file for reading. " - "Error %d.\n", error); + printf("Failed to open file %s for reading. " + "Error %d.\n", filename, error); + printf("Source is %s.\n", (srcdir == NULL) ? "NOT Defined" : srcdir); ini_config_destroy(ini_config); simplebuffer_free(sbobj); return error; @@ -628,6 +657,8 @@ int merge_section_test(void) INIOUT(printf("Comparison of %s %s returned: %d\n", resname, checkname, error)); + INIOUT(printf("<==== Merge section test end ====>\n")); + return error; } @@ -637,23 +668,47 @@ int startup_test(void) struct ini_cfgfile *file_ctx = NULL; struct ini_cfgobj *ini_config = NULL; char **error_list = NULL; - char filename[PATH_MAX]; + char infile[PATH_MAX]; + char outfile[PATH_MAX]; + char command[PATH_MAX * 2 + 3]; + char *srcdir = NULL; + char *builddir; - memcpy(filename, FOO_CONF, sizeof(FOO_CONF)); + INIOUT(printf("<==== Startup test ====>\n")); - /* Ensure that we start with the correct file mode */ - chmod(filename, 0664); + srcdir = getenv("srcdir"); + sprintf(infile, "%s/ini/ini.d/foo.conf", (srcdir == NULL) ? "." : srcdir); + builddir = getenv("builddir"); + sprintf(outfile, "%s/foo.conf", + (builddir == NULL) ? "." : builddir); - INIOUT(printf("<==== Startup test ====>\n")); + sprintf(command, "cp %s %s", infile, outfile); + INIOUT(printf("Running command '%s'\n", command)); + + error = system(command); + if(error) { + printf("Failed to run copy command %d.\n", error); + return error; + } + + sprintf(command, "chmod 600 %s", outfile); + INIOUT(printf("Running command '%s'\n", command)); + + error = system(command); + if(error) { + printf("Failed to run chmod command %d.\n", error); + return error; + } /* Open config file */ - error = ini_config_file_open(filename, + error = ini_config_file_open(outfile, INI_STOP_ON_NONE, 0, INI_META_STATS, &file_ctx); if (error) { - printf("Failed to open file for reading. Error %d.\n", error); + printf("Failed to open %s file for reading. Error %d.\n", + outfile, error); return error; } @@ -667,7 +722,7 @@ int startup_test(void) 0440, /* Checking for r--r----- */ 0); /* This check is expected to fail since - * the actual permissions on the test file are: rw-rw-r-- + * the actual permissions on the test file are: rw------- */ if (!error) { @@ -683,11 +738,11 @@ int startup_test(void) */ 0, /* <- will be real uid in real case */ 0, /* <- will be real gid in real case */ - 0664, /* Checkling for rw-rw-r-- */ + 0600, /* Checkling for rw------- */ 0); if (error) { - printf("Access check failed: %d!\n", error); + printf("Access check failed %d!\n", error); ini_config_file_destroy(file_ctx); return EACCES; } @@ -721,6 +776,8 @@ int startup_test(void) ini_config_destroy(ini_config); + INIOUT(printf("<==== Startup test end ====>\n")); + return 0; } @@ -731,26 +788,37 @@ int reload_test(void) struct ini_cfgfile *file_ctx_new = NULL; char infile[PATH_MAX]; char outfile[PATH_MAX]; - const char *command = "cp"; + char command[PATH_MAX * 2 + 3]; + char *srcdir; char *builddir; int changed = 0; INIOUT(printf("<==== Reload test ====>\n")); - memcpy(infile, FOO_CONF, sizeof(FOO_CONF)); + srcdir = getenv("srcdir"); + sprintf(infile, "%s/ini/ini.d/foo.conf", (srcdir == NULL) ? "." : srcdir); builddir = getenv("builddir"); sprintf(outfile, "%s/foo.conf", (builddir == NULL) ? "." : builddir); - INIOUT(printf("Running command 'cp %s %s'\n", infile, outfile)); + sprintf(command, "cp %s %s", infile, outfile); + INIOUT(printf("Running command '%s'\n", command)); - errno = 0; - if(execlp(command, command, infile, outfile, NULL)) { - error = errno; + error = system(command); + if(error) { printf("Failed to run copy command %d.\n", error); return error; } + sprintf(command, "chmod 600 %s", outfile); + INIOUT(printf("Running command '%s'\n", command)); + + error = system(command); + if(error) { + printf("Failed to run chmod command %d.\n", error); + return error; + } + INIOUT(printf("About to open file: %s'\n", outfile)); /* Open config file */ @@ -760,7 +828,7 @@ int reload_test(void) INI_META_STATS, &file_ctx); if (error) { - printf("Failed to open file for reading. Error %d.\n", error); + printf("Failed to open file %s for reading. Error %d.\n", outfile, error); return error; } @@ -773,7 +841,7 @@ int reload_test(void) */ 0, /* <- will be real uid in real case */ 0, /* <- will be real gid in real case */ - 0664, /* Checkling for rw-rw-r-- */ + 0600, /* Checkling for rw------- */ 0); if (error) { @@ -795,7 +863,7 @@ int reload_test(void) /* Some time passed and we received a signal to reload... */ error = ini_config_file_reopen(file_ctx, &file_ctx_new); if (error) { - printf("Failed to open file for reading. Error %d.\n", error); + printf("Failed to re-open file for reading. Error %d.\n", error); ini_config_file_destroy(file_ctx); return error; } @@ -816,6 +884,8 @@ int reload_test(void) /* Check if file changed */ if (changed) { printf("File changed when it shouldn't. This is unexpected error.\n"); + ini_config_file_print(file_ctx); + ini_config_file_print(file_ctx_new); ini_config_file_destroy(file_ctx); ini_config_file_destroy(file_ctx_new); return EINVAL; @@ -837,11 +907,13 @@ int reload_test(void) return error; } - INIOUT(printf("Copy file again with command 'cp %s %s'\n", infile, outfile)); + sleep(1); - errno = 0; - if (execlp(command, command, infile, outfile, NULL)) { - error = errno; + sprintf(command, "cp %s %s", infile, outfile); + INIOUT(printf("Copy file again with command '%s'\n", command)); + + error = system(command); + if(error) { printf("Failed to run copy command %d.\n", error); ini_config_file_destroy(file_ctx); return error; @@ -853,7 +925,7 @@ int reload_test(void) file_ctx_new = NULL; error = ini_config_file_reopen(file_ctx, &file_ctx_new); if (error) { - printf("Failed to open file for reading. Error %d.\n", error); + printf("Failed to re-open file for reading. Error %d.\n", error); ini_config_file_destroy(file_ctx); return error; } @@ -871,15 +943,21 @@ int reload_test(void) return error; } + INIOUT(printf("Changed value is %d.\n", changed)); + /* Check if file changed */ if (!changed) { printf("File did not change when it should. This is an error.\n"); + ini_config_file_print(file_ctx); + ini_config_file_print(file_ctx_new); ini_config_file_destroy(file_ctx); ini_config_file_destroy(file_ctx_new); return EINVAL; } INIOUT(printf("File changed!\n")); + INIOUT(ini_config_file_print(file_ctx)); + INIOUT(ini_config_file_print(file_ctx_new)); /* We do not need original context any more. */ ini_config_file_destroy(file_ctx); @@ -892,6 +970,7 @@ int reload_test(void) ini_config_file_destroy(file_ctx); + INIOUT(printf("<==== Reload test end ====>\n")); return 0; } @@ -922,6 +1001,7 @@ int main(int argc, char *argv[]) while ((t = tests[i++])) { error = t(); + fflush(NULL); if (error) { INIOUT(printf("Failed with error %d!\n", error)); return error; -- 1.7.1