From 9d1c668008244d6df24ea1a65e814ef244aa09f8 Mon Sep 17 00:00:00 2001 From: Oscar Benjamin Date: Sat, 5 Aug 2023 19:09:01 +0100 Subject: [PATCH 1/5] Remove TMPDIR and use Win API on Windows --- .github/workflows/CI.yml | 4 +--- CMake/cmake_config.h.in | 2 -- CMakeLists.txt | 7 ------- configure.ac | 9 --------- src/qsieve/factor.c | 40 ++++++++++++++++++++-------------------- src/qsieve/init.c | 10 ++++------ 6 files changed, 25 insertions(+), 47 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 695a9e765b..7f1c15cf09 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -470,9 +470,7 @@ jobs: - name: "Configure" run: | ./bootstrap.sh - # We need to specify TMPDIR as we do not have write access to the - # CI's /tmp directory. - ./configure CC=${CC} CFLAGS="${CFLAGS}" TMPDIR="." ${EXTRA_OPTIONS} + ./configure CC=${CC} CFLAGS="${CFLAGS}" ${EXTRA_OPTIONS} - name: "Compile library" run: | diff --git a/CMake/cmake_config.h.in b/CMake/cmake_config.h.in index b3767bb695..b92e0cdf6f 100644 --- a/CMake/cmake_config.h.in +++ b/CMake/cmake_config.h.in @@ -18,8 +18,6 @@ #cmakedefine01 FLINT_USES_FENV -#define FLINT_TMPDIR "@FLINT_TMPDIR@" - #ifdef _MSC_VER #define access _access #define strcasecmp _stricmp diff --git a/CMakeLists.txt b/CMakeLists.txt index 73ca6839e2..11a88d4336 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -54,13 +54,6 @@ if(CMAKE_BUILD_TYPE STREQUAL Debug) set(FLINT_WANT_ASSERT ON) endif() -# temporary directory -if(NOT DEFINED TMPDIR) - set(FLINT_TMPDIR "/tmp" CACHE STRING "Path to a directory meant for temporary files for host system. Only relevant if host do not have read and write access to /tmp [default=/tmp].") -else() - set(FLINT_TMPDIR "${TMPDIR}" CACHE STRING "Path to a directory meant for temporary files for host system. Only relevant if host do not have read and write access to /tmp [default=/tmp].") -endif() - # pthread configuration if(MSVC) diff --git a/configure.ac b/configure.ac index d2b26950d3..17097f25e6 100644 --- a/configure.ac +++ b/configure.ac @@ -475,15 +475,6 @@ AC_ARG_VAR(ABI, [Desired ABI]) AC_ARG_VAR(LDCONFIG, [ldconfig tool]) -AC_ARG_VAR(TMPDIR, [Specify directory meant for temporary files for host system. Only relevant if host do not have read and write access to /tmp [default=/tmp].]) - -if test -z "$TMPDIR"; -then - TMPDIR="/tmp" -fi - -AC_DEFINE_UNQUOTED([FLINT_TMPDIR], ["$TMPDIR"], [Define to set the default directory for temporary files]) - ################################################################################ # check programs and system ################################################################################ diff --git a/src/qsieve/factor.c b/src/qsieve/factor.c index 38cd212afa..9f42c45d2a 100644 --- a/src/qsieve/factor.c +++ b/src/qsieve/factor.c @@ -24,6 +24,11 @@ #undef __STRICT_ANSI__ #endif +/* Use Windows API for temporary files under MSVC */ +#if (defined(__WIN32) && !defined(__CYGWIN__)) || defined(_MSC_VER) +#include +#endif + #include #include #include "thread_support.h" @@ -58,10 +63,8 @@ void qsieve_factor(fmpz_factor_t factors, const fmpz_t n) fmpz_t temp, temp2, X, Y; slong num_facs; fmpz * facs; -#if (defined(__WIN32) && !defined(__CYGWIN__) && !defined(__MINGW32__) && !defined(__MINGW64__)) || defined(_MSC_VER) - const char * tmpnam_ret; -#else - int fd; +#if (defined(__WIN32) && !defined(__CYGWIN__)) || defined(_MSC_VER) + char temp_path[MAX_PATH]; #endif if (fmpz_sgn(n) < 0) @@ -209,24 +212,21 @@ void qsieve_factor(fmpz_factor_t factors, const fmpz_t n) pthread_mutex_init(&qs_inf->mutex, NULL); #endif -#if (defined(__WIN32) && !defined(__CYGWIN__) && !defined(__MINGW32__) && !defined(__MINGW64__)) || defined(_MSC_VER) - tmpnam_ret = tmpnam(NULL); - if (tmpnam_ret == NULL) - flint_throw(FLINT_ERROR, "tmpnam failed\n"); - - strcpy(qs_inf->fname, tmpnam_ret); +#if (defined(__WIN32) && !defined(__CYGWIN__)) || defined(_MSC_VER) + if (GetTempPathA(MAX_PATH, temp_path) == 0) + { + flint_printf("Exception (qsieve_factor). GetTempPathA() failed.\n"); + flint_abort(); + } + if (GetTempFileNameA(temp_path, "siqs", /*uUnique*/ TRUE, qs_inf->fname) == 0) + { + flint_printf("Exception (qsieve_factor). GetTempFileNameA() failed.\n"); + flint_abort(); + } qs_inf->siqs = fopen(qs_inf->fname, "w"); - if (qs_inf->siqs == NULL) - flint_throw(FLINT_ERROR, "fopen failed\n"); #else - strcpy(qs_inf->fname, FLINT_TMPDIR "/siqsXXXXXX"); - fd = mkstemp(qs_inf->fname); - if (fd == -1) - flint_throw(FLINT_ERROR, "mkstemp failed\n"); - - qs_inf->siqs = (FLINT_FILE *) fdopen(fd, "w"); - if (qs_inf->siqs == NULL) - flint_throw(FLINT_ERROR, "fdopen failed\n"); + strcpy(qs_inf->fname, "/tmp/siqsXXXXXX"); /* must be shorter than fname_alloc_size in init.c */ + qs_inf->siqs = (FLINT_FILE *) fdopen(mkstemp(qs_inf->fname), "w"); #endif for (j = qs_inf->small_primes; j < qs_inf->num_primes; j++) diff --git a/src/qsieve/init.c b/src/qsieve/init.c index 269dff54e8..1c5b162a27 100644 --- a/src/qsieve/init.c +++ b/src/qsieve/init.c @@ -13,10 +13,8 @@ #include "fmpz.h" #include "qsieve.h" -#if (defined(__WIN32) && !defined(__CYGWIN__) && !defined(__MINGW32__) && !defined(__MINGW64__)) || defined(_MSC_VER) -# ifndef MAX_PATH -# define MAX_PATH 260 -# endif +#if (defined(__WIN32) && !defined(__CYGWIN__)) || defined(_MSC_VER) +#include #endif void qsieve_init(qs_t qs_inf, const fmpz_t n) @@ -24,10 +22,10 @@ void qsieve_init(qs_t qs_inf, const fmpz_t n) size_t fname_alloc_size; slong i; -#if (defined(__WIN32) && !defined(__CYGWIN__) && !defined(__MINGW32__) && !defined(__MINGW64__)) || defined(_MSC_VER) +#if (defined(__WIN32) && !defined(__CYGWIN__)) || defined(_MSC_VER) fname_alloc_size = MAX_PATH; #else - fname_alloc_size = sizeof(FLINT_TMPDIR "/siqsXXXXXX"); + fname_alloc_size = 20; #endif qs_inf->fname = (char *) flint_malloc(fname_alloc_size); /* space for filename */ From 1a839ae87b69a0a996d957ff0af45d88a4e34752 Mon Sep 17 00:00:00 2001 From: Oscar Benjamin Date: Sun, 6 Aug 2023 16:58:09 +0100 Subject: [PATCH 2/5] Ensure that the file is always closed in qsieve/factor --- src/qsieve/factor.c | 36 +++++++++++++++++++++++++------- src/qsieve/large_prime_variant.c | 12 ++++++++++- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/qsieve/factor.c b/src/qsieve/factor.c index 9f42c45d2a..9c18d8cce4 100644 --- a/src/qsieve/factor.c +++ b/src/qsieve/factor.c @@ -24,11 +24,6 @@ #undef __STRICT_ANSI__ #endif -/* Use Windows API for temporary files under MSVC */ -#if (defined(__WIN32) && !defined(__CYGWIN__)) || defined(_MSC_VER) -#include -#endif - #include #include #include "thread_support.h" @@ -37,6 +32,11 @@ #include "fmpz_vec.h" #include "qsieve.h" +/* Use Windows API for temporary files under MSVC and MinGW */ +#if (defined(__WIN32) && !defined(__CYGWIN__)) || defined(_MSC_VER) +#include +#endif + int compare_facs(const void * a, const void * b) { fmpz * x = (fmpz *) a; @@ -65,6 +65,8 @@ void qsieve_factor(fmpz_factor_t factors, const fmpz_t n) fmpz * facs; #if (defined(__WIN32) && !defined(__CYGWIN__)) || defined(_MSC_VER) char temp_path[MAX_PATH]; +#else + int fd; #endif if (fmpz_sgn(n) < 0) @@ -223,11 +225,29 @@ void qsieve_factor(fmpz_factor_t factors, const fmpz_t n) flint_printf("Exception (qsieve_factor). GetTempFileNameA() failed.\n"); flint_abort(); } - qs_inf->siqs = fopen(qs_inf->fname, "w"); + qs_inf->siqs = (FLINT_FILE *) fopen(qs_inf->fname, "w"); + if (qs_inf->siqs == NULL) + flint_throw(FLINT_ERROR, "fopen failed\n"); #else strcpy(qs_inf->fname, "/tmp/siqsXXXXXX"); /* must be shorter than fname_alloc_size in init.c */ - qs_inf->siqs = (FLINT_FILE *) fdopen(mkstemp(qs_inf->fname), "w"); + fd = mkstemp(qs_inf->fname); + if (fd == -1) + flint_throw(FLINT_ERROR, "mkstemp failed\n"); + + qs_inf->siqs = (FLINT_FILE *) fdopen(fd, "w"); + if (qs_inf->siqs == NULL) + flint_throw(FLINT_ERROR, "fdopen failed\n"); #endif + /* + * The code here and in large_prime_variant.c opens and closes the file + * qs_inf->fname in several different places. On Windows all file handles + * need to be closed before the file can be removed in cleanup at function + * exit. The invariant that needs to be preserved at each open/close is + * that either + * qs_inf->siqs is NULL and there are no open handles to the file, + * or + * qs_inf->siqs is not NULL and is the *only* open handle to the file. + */ for (j = qs_inf->small_primes; j < qs_inf->num_primes; j++) { @@ -465,6 +485,8 @@ void qsieve_factor(fmpz_factor_t factors, const fmpz_t n) flint_give_back_threads(qs_inf->handles, qs_inf->num_handles); flint_free(sieve); + if (qs_inf->siqs != NULL && fclose((FILE *) qs_inf->siqs)) + flint_throw(FLINT_ERROR, "fclose fail\n"); if (remove(qs_inf->fname)) flint_throw(FLINT_ERROR, "remove fail\n"); qsieve_clear(qs_inf); diff --git a/src/qsieve/large_prime_variant.c b/src/qsieve/large_prime_variant.c index 4a1989b34e..a8f4849f6d 100644 --- a/src/qsieve/large_prime_variant.c +++ b/src/qsieve/large_prime_variant.c @@ -503,7 +503,11 @@ int qsieve_process_relation(qs_t qs_inf) relation_t * rlist; int done = 0; + if (qs_inf->siqs != NULL && fclose((FILE *) qs_inf->siqs)) + flint_throw(FLINT_ERROR, "fclose fail\n"); qs_inf->siqs = (FLINT_FILE *) fopen(qs_inf->fname, "r"); + if (qs_inf->siqs == NULL) + flint_throw(FLINT_ERROR, "fopen fail\n"); #if QS_DEBUG & 64 printf("Getting relations\n"); @@ -528,7 +532,9 @@ int qsieve_process_relation(qs_t qs_inf) } } - fclose((FILE *) qs_inf->siqs); + if(fclose((FILE *) qs_inf->siqs)) + flint_throw(FLINT_ERROR, "fclose fail\n"); + qs_inf->siqs = NULL; #if QS_DEBUG & 64 printf("Removing duplicates\n"); @@ -581,7 +587,11 @@ int qsieve_process_relation(qs_t qs_inf) { qs_inf->edges -= 100; done = 0; + if (qs_inf->siqs != NULL && fclose((FILE *) qs_inf->siqs)) + flint_throw(FLINT_ERROR, "fclose fail\n"); qs_inf->siqs = (FLINT_FILE *) fopen(qs_inf->fname, "a"); + if (qs_inf->siqs == NULL) + flint_throw(FLINT_ERROR, "fopen fail\n"); } else { done = 1; From 12026b49ef9c52322a89231bc3ea14d562fde5da Mon Sep 17 00:00:00 2001 From: Oscar Benjamin Date: Sun, 6 Aug 2023 18:15:02 +0100 Subject: [PATCH 3/5] Add debug output in qsieve/factor.c --- src/qsieve/factor.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qsieve/factor.c b/src/qsieve/factor.c index 9c18d8cce4..7852de9d93 100644 --- a/src/qsieve/factor.c +++ b/src/qsieve/factor.c @@ -487,8 +487,12 @@ void qsieve_factor(fmpz_factor_t factors, const fmpz_t n) flint_free(sieve); if (qs_inf->siqs != NULL && fclose((FILE *) qs_inf->siqs)) flint_throw(FLINT_ERROR, "fclose fail\n"); - if (remove(qs_inf->fname)) + if (remove(qs_inf->fname)) { + perror("qsieve/factor.c: remove failed. perror says"); + printf("qs_inf->fname is %s\n", qs_inf->fname); + printf("qs_inf->siqs != NULL is %d\n", qs_inf->siqs != NULL); flint_throw(FLINT_ERROR, "remove fail\n"); + } qsieve_clear(qs_inf); qsieve_linalg_clear(qs_inf); qsieve_poly_clear(qs_inf); From 26bdd65263aab7cbfb48c2ed614d21a4d6cd6da3 Mon Sep 17 00:00:00 2001 From: Oscar Benjamin Date: Sun, 6 Aug 2023 19:18:05 +0100 Subject: [PATCH 4/5] Use uUnique=0 in GetTempFileNameA --- src/qsieve/factor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qsieve/factor.c b/src/qsieve/factor.c index 7852de9d93..530a2f52da 100644 --- a/src/qsieve/factor.c +++ b/src/qsieve/factor.c @@ -220,7 +220,8 @@ void qsieve_factor(fmpz_factor_t factors, const fmpz_t n) flint_printf("Exception (qsieve_factor). GetTempPathA() failed.\n"); flint_abort(); } - if (GetTempFileNameA(temp_path, "siqs", /*uUnique*/ TRUE, qs_inf->fname) == 0) + /* uUnique = 0 means the we *do* want a unique filename (obviously!). */ + if (GetTempFileNameA(temp_path, "siq", /*uUnique*/ 0, qs_inf->fname) == 0) { flint_printf("Exception (qsieve_factor). GetTempFileNameA() failed.\n"); flint_abort(); From 7df0933c4244ede772dcbcab27afe53a8c57be27 Mon Sep 17 00:00:00 2001 From: Oscar Benjamin Date: Sun, 6 Aug 2023 20:31:43 +0100 Subject: [PATCH 5/5] Remove debug output for CI --- src/qsieve/factor.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/qsieve/factor.c b/src/qsieve/factor.c index 530a2f52da..626526887c 100644 --- a/src/qsieve/factor.c +++ b/src/qsieve/factor.c @@ -489,9 +489,6 @@ void qsieve_factor(fmpz_factor_t factors, const fmpz_t n) if (qs_inf->siqs != NULL && fclose((FILE *) qs_inf->siqs)) flint_throw(FLINT_ERROR, "fclose fail\n"); if (remove(qs_inf->fname)) { - perror("qsieve/factor.c: remove failed. perror says"); - printf("qs_inf->fname is %s\n", qs_inf->fname); - printf("qs_inf->siqs != NULL is %d\n", qs_inf->siqs != NULL); flint_throw(FLINT_ERROR, "remove fail\n"); } qsieve_clear(qs_inf);