tests: Generalize temp dir management

Currently, afstest_BuildTestConfig calls afstest_mkdtemp (our thin
wrapper around mkdtemp) to create its temporary config dir. We may
want to make new tests, though, that create a temp dir for other
purposes. To make that easier, move a little more code into
afstest_mkdtemp, so the caller doesn't need to construct the template.

To allow callers to clean up such temporary dirs, change
afstest_UnlinkTestConfig into a more general function,
afstest_rmdtemp. Allow this new function to remove all files in a dir,
not just files one-level-deep. To avoid needing to write our own
traversal and removal logic, just run 'rm -rf' via a new function,
afstest_systemlp().

Move these temp dir-related functions from config.c into files.c,
since they are no longer specific to config dirs.

Change-Id: I16750a2f30e98c9ca2e14dfb7d3fc9bc5d456e8d
Reviewed-on: https://gerrit.openafs.org/14632
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

diff --git a/tests/auth/authcon-t.c b/tests/auth/authcon-t.c
index 0107b16..bdfd256 100644 (file)
--- a/tests/auth/authcon-t.c
+++ b/tests/auth/authcon-t.c
@@ -92,6 +92,6 @@ main(int argc, char **argv)
     ok(afsconf_UpToDate(dir), "afsconf_GetLatestKeyByTypes resest UpToDate");
 
 out:
-    afstest_UnlinkTestConfig(dirname);
+    afstest_rmdtemp(dirname);
     return code;
 }

diff --git a/tests/auth/keys-t.c b/tests/auth/keys-t.c
index 50f3ab5..e814080 100644 (file)
--- a/tests/auth/keys-t.c
+++ b/tests/auth/keys-t.c
@@ -561,7 +561,7 @@ int main(int argc, char **argv)
 
     afsconf_Close(dir);
 
-    afstest_UnlinkTestConfig(dirname);
+    afstest_rmdtemp(dirname);
     free(dirname);
     free(keyfile);
 
@@ -587,7 +587,7 @@ int main(int argc, char **argv)
        " ... with the right key");
 
 out:
-    afstest_UnlinkTestConfig(dirname);
+    afstest_rmdtemp(dirname);
 
     return 0;
 }

diff --git a/tests/auth/realms-t.c b/tests/auth/realms-t.c
index 04fac4e..23290d5 100644 (file)
--- a/tests/auth/realms-t.c
+++ b/tests/auth/realms-t.c
@@ -258,7 +258,7 @@ test_edges(void)
        exit(1);
     }
     run_edge_tests(dir);
-    afstest_UnlinkTestConfig(dirname);
+    afstest_rmdtemp(dirname);
 }
 
 void
@@ -275,7 +275,7 @@ test_no_config_files(void)
        exit(1);
     }
     run_tests(dir, 0, "no config");
-    afstest_UnlinkTestConfig(dirname);
+    afstest_rmdtemp(dirname);
 }
 
 void
@@ -294,7 +294,7 @@ test_with_config_files(void)
        exit(1);
     }
     run_tests(dir, 2, "config");
-    afstest_UnlinkTestConfig(dirname);
+    afstest_rmdtemp(dirname);
 }
 
 void
@@ -317,7 +317,7 @@ test_set_local_realms(void)
     }
     write_krb_conf(dirname, "SOME.REALM.ORG");
     run_tests(dir, 1, "set realm test");
-    afstest_UnlinkTestConfig(dirname);
+    afstest_rmdtemp(dirname);
 }
 
 void
@@ -353,7 +353,7 @@ test_update_config_files(void)
     code = afsconf_IsLocalRealmMatch(dir, &local, "admin", NULL, "MY.REALM.ORG");
     ok(code == 0 && local == 0, "after update: admin@MY.REALM.ORG");
 
-    afstest_UnlinkTestConfig(dirname);
+    afstest_rmdtemp(dirname);
 }
 
 int

diff --git a/tests/auth/superuser-t.c b/tests/auth/superuser-t.c
index 7185722..52c1902 100644 (file)
--- a/tests/auth/superuser-t.c
+++ b/tests/auth/superuser-t.c
@@ -489,7 +489,7 @@ int main(int argc, char **argv)
 
 out:
     /* Client and server are both done, so cleanup after everything */
-    afstest_UnlinkTestConfig(dirname);
+    afstest_rmdtemp(dirname);
 
     return ret;
 }

diff --git a/tests/auth/writekeyfile.c b/tests/auth/writekeyfile.c
index 50eb399..918bb2a 100644 (file)
--- a/tests/auth/writekeyfile.c
+++ b/tests/auth/writekeyfile.c
@@ -65,7 +65,7 @@ main(int argc, char **argv)
     close(in);
     close(out);
 
-    afstest_UnlinkTestConfig(dirname);
+    afstest_rmdtemp(dirname);
 
     return 0;
 }

diff --git a/tests/common/common.h b/tests/common/common.h
index 7d43782..921b366 100644 (file)
--- a/tests/common/common.h
+++ b/tests/common/common.h
@@ -24,8 +24,6 @@
 
 /* config.c */
 extern char *afstest_BuildTestConfig(void);
-extern void afstest_UnlinkTestConfig(char *);
-extern char *afstest_mkdtemp(char *template);
 
 struct afsconf_dir;
 extern int afstest_AddDESKeyFile(struct afsconf_dir *dir);
@@ -48,9 +46,12 @@ struct afstest_cmdinfo {
 extern int is_command(struct afstest_cmdinfo *cmdinfo,
                      const char *format, ...)
        AFS_ATTRIBUTE_FORMAT(__printf__, 2, 3);
+extern int afstest_systemlp(char *command, ...);
 
 /* files.c */
 
+extern char *afstest_mkdtemp(void);
+extern void afstest_rmdtemp(char *path);
 extern char *afstest_src_path(char *path);
 extern char *afstest_obj_path(char *path);
 

diff --git a/tests/common/config.c b/tests/common/config.c
index a678fe4..9841639 100644 (file)
--- a/tests/common/config.c
+++ b/tests/common/config.c
@@ -31,7 +31,6 @@
 #include <roken.h>
 
 #include <afs/cellconfig.h>
-#include <afs/afsutil.h>
 
 #include <hcrypto/des.h>
 
@@ -50,39 +49,6 @@ openConfigFile(char *dirname, char *filename) {
     return file;
 }
 
-static void
-unlinkConfigFile(char *dirname, char *filename) {
-    char *path;
-
-    path = afstest_asprintf("%s/%s", dirname, filename);
-    unlink(path);
-    free(path);
-}
-
-/*!
- *  Wrapper for mkdtemp
- */
-
-char *
-afstest_mkdtemp(char *template)
-{
-#if defined(HAVE_MKDTEMP)
-    return mkdtemp(template);
-#else
-    /*
-     * Note that using the following is not a robust replacement
-     * for mkdtemp as there is a possible race condition between
-     * creating the name and creating the directory itself.  The
-     * use of this routine is limited to running tests.
-     */
-    if (mktemp(template) == NULL)
-       return NULL;
-    if (mkdir(template, 0700))
-       return NULL;
-    return template;
-#endif
-}
-
 /*!
  * Build a test configuration directory, containing a CellServDB and ThisCell
  * file for the "example.org" cell
@@ -101,10 +67,10 @@ afstest_BuildTestConfig(void) {
     char hostname[255];
     struct in_addr iaddr;
 
-    dir = afstest_asprintf("%s/afs_XXXXXX", gettmpdir());
-
-    if (afstest_mkdtemp(dir) == NULL)
+    dir = afstest_mkdtemp();
+    if (dir == NULL) {
        goto fail;
+    }
 
     /* Work out which IP address to use in our CellServDB. We figure this out
      * according to the IP address which ubik is most likely to pick for one of
@@ -135,29 +101,6 @@ fail:
     return NULL;
 }
 
-/*!
- * Delete at test configuration directory
- */
-
-void
-afstest_UnlinkTestConfig(char *dir)
-{
-    DIR *dirp;
-    struct dirent *de;
-
-    /* Sanity check, only zap directories that look like ours */
-    if (!strstr(dir, "afs_"))
-       return;
-    if (getenv("MAKECHECK") != NULL) {
-       dirp = opendir(dir);
-       if (!dirp)
-           return;
-       while ((de = readdir(dirp)))
-           unlinkConfigFile(dir, de->d_name);
-       rmdir(dir);
-    }
-}
-
 int
 afstest_AddDESKeyFile(struct afsconf_dir *dir)
 {

diff --git a/tests/common/exec.c b/tests/common/exec.c
index 7670cd0..f5c93fb 100644 (file)
--- a/tests/common/exec.c
+++ b/tests/common/exec.c
@@ -32,6 +32,8 @@
 #include <roken.h>
 #include <afs/opr.h>
 
+#include <afs/opr.h>
+
 #include <sys/types.h>
 #include <sys/wait.h>
 
@@ -196,3 +198,79 @@ is_command(struct afstest_cmdinfo *cmdinfo, const char *format, ...)
 
     return ret;
 }
+
+static int
+do_systemvp(char *command, char **argv)
+{
+    pid_t child;
+
+    child = fork();
+    if (child < 0) {
+       sysbail("fork");
+    }
+
+    if (child > 0) {
+       int status = -1;
+       /* parent */
+       if (waitpid(child, &status, 0) <= 0) {
+           sysbail("waitpid");
+       }
+       return status;
+    }
+
+    execvp(command, argv);
+    sysbail("execvp");
+    exit(1);
+}
+
+/*
+ * This function is mostly the same as system(), in that the given command is
+ * run with the same stdout, stderr, etc, and we return the exit status when it
+ * finishes running. But instead of giving the command as a single string,
+ * interpreted by the shell, the command is given as a list of arguments, like
+ * execlp().
+ *
+ * For example, the following two are equivalent:
+ *
+ * code = afstest_systemlp("echo", "foo", "bar", (char*)NULL);
+ * code = system("echo foo bar");
+ *
+ * Except that with afstest_systemlp, the arguments don't go through the shell,
+ * so you don't need to worry about quoting arguments or escaping shell
+ * metacharacters.
+ */
+int
+afstest_systemlp(char *command, ...)
+{
+    va_list ap;
+    int n_args = 1;
+    int arg_i;
+    int status;
+    char **argv;
+
+    va_start(ap, command);
+    while (va_arg(ap, char*) != NULL) {
+       n_args++;
+    }
+    va_end(ap);
+
+    argv = calloc(n_args + 1, sizeof(argv[0]));
+    opr_Assert(argv != NULL);
+
+    argv[0] = command;
+
+    va_start(ap, command);
+    for (arg_i = 1; arg_i < n_args; arg_i++) {
+       argv[arg_i] = va_arg(ap, char*);
+       opr_Assert(argv[arg_i] != NULL);
+    }
+    va_end(ap);
+
+    /* Note that argv[n_args] is NULL, since we allocated an extra arg in
+     * calloc(), above. */
+
+    status = do_systemvp(command, argv);
+    free(argv);
+
+    return status;
+}

diff --git a/tests/common/files.c b/tests/common/files.c
index 6c13edd..ecb96c6 100644 (file)
--- a/tests/common/files.c
+++ b/tests/common/files.c
@@ -37,6 +37,58 @@
 
 #include "common.h"
 
+char *
+afstest_mkdtemp(void)
+{
+    char *template;
+
+    template = afstest_asprintf("%s/afs_XXXXXX", gettmpdir());
+
+#if defined(HAVE_MKDTEMP)
+    return mkdtemp(template);
+#else
+    /*
+     * Note that using the following is not a robust replacement
+     * for mkdtemp as there is a possible race condition between
+     * creating the name and creating the directory itself.  The
+     * use of this routine is limited to running tests.
+     */
+    if (mktemp(template) == NULL)
+       return NULL;
+    if (mkdir(template, 0700))
+       return NULL;
+    return template;
+#endif
+}
+
+void
+afstest_rmdtemp(char *path)
+{
+    int code;
+    struct stat st;
+
+    /* Sanity check, only zap directories that look like ours */
+    opr_Assert(strstr(path, "afs_") != NULL);
+    if (getenv("MAKECHECK") == NULL) {
+       /* Don't delete tmp dirs if we're not running under 'make check'. */
+       return;
+    }
+    code = lstat(path, &st);
+    if (code != 0) {
+       /* Given path doesn't exist (or we can't access it?) */
+       return;
+    }
+    if (!S_ISDIR(st.st_mode)) {
+       /* Path isn't a dir; that's weird. Bail out to be on the safe side. */
+       return;
+    }
+    afstest_systemlp("rm", "-rf",
+#if defined(AFS_LINUX_ENV) || defined(AFS_SUN511_ENV)
+                    "--one-file-system",
+#endif
+                    path, (char*)NULL);
+}
+
 static char *
 path_from_tdir(char *env_var, char *filename)
 {

diff --git a/tests/volser/vos-t.c b/tests/volser/vos-t.c
index c58a651..f00fdbf 100644 (file)
--- a/tests/volser/vos-t.c
+++ b/tests/volser/vos-t.c
@@ -139,6 +139,6 @@ out:
        is_int(0, code, "Server exited cleanly");
     }
 
-    afstest_UnlinkTestConfig(dirname);
+    afstest_rmdtemp(dirname);
     return ret;
 }