Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Some temporary directory handling changes #94

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Some temporary directory handling changes #94

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3b35aff
canonicalize temp sandbox directory everywhere
ethomson Apr 29, 2024
ca684b9
tempdir and sandboxes are now separate entities
ethomson Apr 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions clar.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,10 @@ static void clar_print_onsuite(const char *suite_name, int suite_index);
static void clar_print_onabort(const char *msg, ...);

/* From clar_sandbox.c */
static void clar_unsandbox(void);
static int clar_sandbox(void);
static int clar_tempdir_init(void);
static void clar_tempdir_shutdown(void);
static int clar_sandbox_create(void);
static int clar_sandbox_cleanup(void);

/* From summary.h */
static struct clar_summary *clar_summary_init(const char *filename);
Expand Down Expand Up @@ -296,6 +298,8 @@ clar_run_test(

CL_TRACE(CL_TRACE__TEST__BEGIN);

clar_sandbox_create();

_clar.last_report->start = time(NULL);
clar_time_now(&start);

Expand Down Expand Up @@ -323,6 +327,8 @@ clar_run_test(
if (cleanup->ptr != NULL)
cleanup->ptr();

clar_sandbox_cleanup();

CL_TRACE(CL_TRACE__TEST__END);

_clar.tests_ran++;
Expand Down Expand Up @@ -578,8 +584,8 @@ clar_test_init(int argc, char **argv)
exit(-1);
}

if (clar_sandbox() < 0) {
clar_print_onabort("Failed to sandbox the test runner.\n");
if (clar_tempdir_init() < 0) {
clar_print_onabort("Failed to create tempdir for the test runner.\n");
exit(-1);
}
}
Expand Down Expand Up @@ -613,7 +619,7 @@ clar_test_shutdown(void)
_clar.total_errors
);

clar_unsandbox();
clar_tempdir_shutdown();

if (_clar.write_summary && clar_summary_shutdown(_clar.summary) < 0) {
clar_print_onabort("Failed to write the summary file\n");
Expand Down
1 change: 1 addition & 0 deletions clar.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ void clar_test_shutdown(void);
int clar_test(int argc, char *argv[]);

const char *clar_sandbox_path(void);
const char *clar_tempdir_path(void);

void cl_set_cleanup(void (*cleanup)(void *), void *opaque);
void cl_fs_cleanup(void);
Expand Down
4 changes: 2 additions & 2 deletions clar/fixtures.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const char *cl_fixture(const char *fixture_name)

void cl_fixture_sandbox(const char *fixture_name)
{
fs_copy(cl_fixture(fixture_name), _clar_path);
fs_copy(cl_fixture(fixture_name), clar_sandbox_path());
}

const char *cl_fixture_basename(const char *fixture_name)
Expand All @@ -45,6 +45,6 @@ const char *cl_fixture_basename(const char *fixture_name)

void cl_fixture_cleanup(const char *fixture_name)
{
fs_rm(fixture_path(_clar_path, cl_fixture_basename(fixture_name)));
fs_rm(fixture_path(clar_sandbox_path(), cl_fixture_basename(fixture_name)));
}
#endif
6 changes: 3 additions & 3 deletions clar/fs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ fs_rm_wait(WCHAR *_wpath)
ERROR_PATH_NOT_FOUND == last_error)
return 0;

Sleep(RM_RETRY_DELAY * retries * retries);
Sleep(RM_RETRY_DELAY * retries * retries);
}
while (retries++ <= RM_RETRY_COUNT);

Expand Down Expand Up @@ -516,7 +516,7 @@ fs_rm(const char *path)
void
cl_fs_cleanup(void)
{
clar_unsandbox();
clar_sandbox();
clar_tempdir_shutdown();
clar_tempdir_init();
}
#endif
142 changes: 113 additions & 29 deletions clar/sandbox.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,17 @@
#include <sys/syslimits.h>
#endif

static char _clar_path[4096 + 1];
/*
* The tempdir is the temporary directory for the entirety of the clar
* process execution. The sandbox is an individual temporary directory
* for the execution of an individual test. Sandboxes are deleted
* entirely after test execution to avoid pollution across tests.
*/
#define CLAR_PATH_MAX 4096
static char _clar_tempdir[CLAR_PATH_MAX];
size_t _clar_tempdir_len;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be non-static? I couldn't spot any uses of it outside of the current scope.


static char _clar_sandbox[CLAR_PATH_MAX];

static int
is_valid_tmp_path(const char *path)
Expand Down Expand Up @@ -35,10 +45,9 @@ find_tmp_path(char *buffer, size_t length)
continue;

if (is_valid_tmp_path(env)) {
#ifdef __APPLE__
if (length >= PATH_MAX && realpath(env, buffer) != NULL)
return 0;
#endif
if (strlen(env) + 1 > CLAR_PATH_MAX)
return -1;

strncpy(buffer, env, length - 1);
buffer[length - 1] = '\0';
return 0;
Expand All @@ -47,10 +56,6 @@ find_tmp_path(char *buffer, size_t length)

/* If the environment doesn't say anything, try to use /tmp */
if (is_valid_tmp_path("/tmp")) {
#ifdef __APPLE__
if (length >= PATH_MAX && realpath("/tmp", buffer) != NULL)
return 0;
#endif
strncpy(buffer, "/tmp", length - 1);
buffer[length - 1] = '\0';
return 0;
Expand All @@ -75,17 +80,45 @@ find_tmp_path(char *buffer, size_t length)
return -1;
}

static void clar_unsandbox(void)
static int canonicalize_tmp_path(char *buffer)
{
#ifdef _WIN32
char tmp[CLAR_PATH_MAX];
DWORD ret;

ret = GetFullPathName(buffer, CLAR_PATH_MAX, tmp, NULL);

if (ret == 0 || ret > CLAR_PATH_MAX)
return -1;

ret = GetLongPathName(tmp, buffer, CLAR_PATH_MAX);

if (ret == 0 || ret > CLAR_PATH_MAX)
return -1;

return 0;
#else
char tmp[CLAR_PATH_MAX];

if (realpath(buffer, tmp) == NULL)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry a bit that realpath may not be available on all systems, as it is only specified by POSIX.1-2001 and thus not part of C90 itself. And unfortunately we have folks that compile Git on systems that predate POSIX.1-2001 itself.

I sometimes wish that some kind of resource existed that provides a matrix of operating system <-> availability of certain features.

return -1;

strcpy(buffer, tmp);
return 0;
#endif
}

static void clar_tempdir_shutdown(void)
{
if (_clar_path[0] == '\0')
if (_clar_tempdir[0] == '\0')
return;

cl_must_pass(chdir(".."));

fs_rm(_clar_path);
fs_rm(_clar_tempdir);
}

static int build_sandbox_path(void)
static int build_tempdir_path(void)
{
#ifdef CLAR_TMPDIR
const char path_tail[] = CLAR_TMPDIR "_XXXXXX";
Expand All @@ -95,60 +128,111 @@ static int build_sandbox_path(void)

size_t len;

if (find_tmp_path(_clar_path, sizeof(_clar_path)) < 0)
if (find_tmp_path(_clar_tempdir, sizeof(_clar_tempdir)) < 0 ||
canonicalize_tmp_path(_clar_tempdir) < 0)
return -1;

len = strlen(_clar_path);
len = strlen(_clar_tempdir);

#ifdef _WIN32
{ /* normalize path to POSIX forward slashes */
size_t i;
for (i = 0; i < len; ++i) {
if (_clar_path[i] == '\\')
_clar_path[i] = '/';
if (_clar_tempdir[i] == '\\')
_clar_tempdir[i] = '/';
}
}
#endif

if (_clar_path[len - 1] != '/') {
_clar_path[len++] = '/';
if (_clar_tempdir[len - 1] != '/') {
_clar_tempdir[len++] = '/';
}

strncpy(_clar_path + len, path_tail, sizeof(_clar_path) - len);
strncpy(_clar_tempdir + len, path_tail, sizeof(_clar_tempdir) - len);

#if defined(__MINGW32__)
if (_mktemp(_clar_path) == NULL)
if (_mktemp(_clar_tempdir) == NULL)
return -1;

if (mkdir(_clar_path, 0700) != 0)
if (mkdir(_clar_tempdir, 0700) != 0)
return -1;
#elif defined(_WIN32)
if (_mktemp_s(_clar_path, sizeof(_clar_path)) != 0)
if (_mktemp_s(_clar_tempdir, sizeof(_clar_tempdir)) != 0)
return -1;

if (mkdir(_clar_path, 0700) != 0)
if (mkdir(_clar_tempdir, 0700) != 0)
return -1;
#else
if (mkdtemp(_clar_path) == NULL)
if (mkdtemp(_clar_tempdir) == NULL)
return -1;
#endif

_clar_tempdir_len = strlen(_clar_tempdir);
return 0;
}

static int clar_sandbox(void)
static int clar_tempdir_init(void)
{
if (_clar_path[0] == '\0' && build_sandbox_path() < 0)
if (_clar_tempdir[0] == '\0' && build_tempdir_path() < 0)
return -1;

if (chdir(_clar_path) != 0)
if (chdir(_clar_tempdir) != 0)
return -1;

srand(clock() ^ time(NULL) ^ (getpid() << 16));

return 0;
}

static int clar_sandbox_create(void)
{
char alpha[] = "0123456789abcdef";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be const, right?

int num = rand();

cl_assert(_clar_sandbox[0] == '\0');

strcpy(_clar_sandbox, _clar_tempdir);
_clar_sandbox[_clar_tempdir_len] = '/';

_clar_sandbox[_clar_tempdir_len + 1] = alpha[(num & 0xf0000000) >> 28];
_clar_sandbox[_clar_tempdir_len + 2] = alpha[(num & 0x0f000000) >> 24];
_clar_sandbox[_clar_tempdir_len + 3] = alpha[(num & 0x00f00000) >> 20];
_clar_sandbox[_clar_tempdir_len + 4] = alpha[(num & 0x000f0000) >> 16];
_clar_sandbox[_clar_tempdir_len + 5] = alpha[(num & 0x0000f000) >> 12];
_clar_sandbox[_clar_tempdir_len + 6] = alpha[(num & 0x00000f00) >> 8];
_clar_sandbox[_clar_tempdir_len + 7] = alpha[(num & 0x000000f0) >> 4];
_clar_sandbox[_clar_tempdir_len + 8] = alpha[(num & 0x0000000f) >> 0];
_clar_sandbox[_clar_tempdir_len + 9] = '\0';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we have a central directory where all test results will be written into, do we need randomness at all? We could compute statically derived paths, e.g. by appending the test name to the sandbox path. These have to be unique anyway, so the only thing that we need to care about is to prune such a directory that may be left over from a previous run.


if (mkdir(_clar_sandbox, 0700) != 0)
return -1;

if (chdir(_clar_sandbox) != 0)
return -1;

return 0;
}

static int clar_sandbox_cleanup(void)
{
cl_assert(_clar_sandbox[0] != '\0');

fs_rm(_clar_sandbox);
_clar_sandbox[0] = '\0';

if (chdir(_clar_tempdir) != 0)
return -1;

return 0;
}

const char *clar_tempdir_path(void)
{
return _clar_tempdir;
}

const char *clar_sandbox_path(void)
{
return _clar_path;
return _clar_sandbox;
}

Loading