public inbox for libstdc++-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc(refs/users/aoliva/heads/testme)] __gnu_test::nonexistent_path: Always include counter in filename returned
@ 2022-06-23  9:03 Alexandre Oliva
  0 siblings, 0 replies; 7+ messages in thread
From: Alexandre Oliva @ 2022-06-23  9:03 UTC (permalink / raw)
  To: gcc-cvs, libstdc++-cvs

https://gcc.gnu.org/g:d477b7478e5025fe45d3cd421fe6c7caac540657

commit d477b7478e5025fe45d3cd421fe6c7caac540657
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Thu Jun 23 05:03:54 2022 -0300

    __gnu_test::nonexistent_path: Always include counter in filename returned
    
    We have noticed that, on RTEMS, a small number of testscases are
    failing because two calls to this method return the same filename.
    This happens for instance in 27_io/filesystem/operations/copy_file.cc
    where it does:
    
      auto from = __gnu_test::nonexistent_path();
      auto to = __gnu_test::nonexistent_path();
    
    We tracked this issue down to the fact that the implementation of
    mkstemp on that system appears to use a very predictable algorithm
    for chosing the name of the temporary file, where the same filename
    appears to be tried in the same order, regardless of past calls.
    So, as long as the file gets deleted after a call to mkstemp (something
    we do here in our nonexistent_path method), the next call to mkstemps
    ends up returning the same filename, causing the collision we se above.
    
    This commit enhances the __gnu_test::nonexistent_path method to
    introduce in the filename being returned a counter which gets
    incremented at every call of this method.
    
    libstdc++-v3/ChangeLog:
    
            * testsuite/util/testsuite_fs.h (__gnu_test::nonexistent_path):
            Always include a counter in the filename returned.

Diff:
---
 libstdc++-v3/testsuite/util/testsuite_fs.h | 31 ++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h
index 037d9ffc0f4..206ea677790 100644
--- a/libstdc++-v3/testsuite/util/testsuite_fs.h
+++ b/libstdc++-v3/testsuite/util/testsuite_fs.h
@@ -38,9 +38,9 @@ namespace test_fs = std::experimental::filesystem;
 
 #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L
 #include <stdlib.h> // mkstemp
-#else
-#include <random>   // std::random_device
+#include <cstring> // strcpy
 #endif
+#include <random>   // std::random_device
 
 #if defined(__MINGW32__) || defined(__MINGW64__) \
   || !defined (_GLIBCXX_HAVE_SYMLINK)
@@ -125,8 +125,32 @@ namespace __gnu_test
       file.erase(0, pos+1);
 
     test_fs::path p;
+    // A counter, starting from a random value, to be included as part
+    // of the filename being returned, and incremented each time
+    // this method is used.  It allows us to ensure that two calls
+    // to this method can never return the same filename, something
+    // testcases do when they need multiple non-existent filenames
+    // for their purposes.
+    static unsigned counter = std::random_device{}();
+
 #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L
-    char tmp[] = "filesystem-test.XXXXXX";
+    // Use mkstemp to determine the name of a file which does not exist yet.
+    //
+    // Note that we have seen on some systems (such as RTEMS, for instance)
+    // that mkstemp behaves very predictably, causing it to always try
+    // the same sequence of file names.  In other words, if we call mkstemp
+    // with a pattern, delete the file it created (which is what we do, here),
+    // and call mkstemp with the same pattern again, it returns the same
+    // filename once more.  While most implementations introduce a degree
+    // of randomness, it is not mandated by the standard, and this is why
+    // we include a counter in the template passed to mkstemp.
+    std::string mkstemp_template ("filesystem-test.");
+    mkstemp_template.append(std::to_string (counter++));
+    mkstemp_template.append(".XXXXXX");
+
+    char tmp[mkstemp_template.length() + 1];
+    std::strcpy (tmp, mkstemp_template.c_str());
+
     int fd = ::mkstemp(tmp);
     if (fd == -1)
       throw test_fs::filesystem_error("mkstemp failed",
@@ -141,7 +165,6 @@ namespace __gnu_test
     if (file.length() > 64)
       file.resize(64);
     char buf[128];
-    static unsigned counter = std::random_device{}();
 #if _GLIBCXX_USE_C99_STDIO
     std::snprintf(buf, 128,
 #else


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [gcc(refs/users/aoliva/heads/testme)] __gnu_test::nonexistent_path: Always include counter in filename returned
@ 2022-06-23 12:44 Alexandre Oliva
  0 siblings, 0 replies; 7+ messages in thread
From: Alexandre Oliva @ 2022-06-23 12:44 UTC (permalink / raw)
  To: gcc-cvs, libstdc++-cvs

https://gcc.gnu.org/g:5f62dd2f6ee29af0cce8d92cd1e4105daa9271f8

commit 5f62dd2f6ee29af0cce8d92cd1e4105daa9271f8
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Thu Jun 23 05:03:54 2022 -0300

    __gnu_test::nonexistent_path: Always include counter in filename returned
    
    We have noticed that, on RTEMS, a small number of testscases are
    failing because two calls to this method return the same filename.
    This happens for instance in 27_io/filesystem/operations/copy_file.cc
    where it does:
    
      auto from = __gnu_test::nonexistent_path();
      auto to = __gnu_test::nonexistent_path();
    
    We tracked this issue down to the fact that the implementation of
    mkstemp on that system appears to use a very predictable algorithm
    for chosing the name of the temporary file, where the same filename
    appears to be tried in the same order, regardless of past calls.
    So, as long as the file gets deleted after a call to mkstemp (something
    we do here in our nonexistent_path method), the next call to mkstemps
    ends up returning the same filename, causing the collision we se above.
    
    This commit enhances the __gnu_test::nonexistent_path method to
    introduce in the filename being returned a counter which gets
    incremented at every call of this method.
    
    libstdc++-v3/ChangeLog:
    
            * testsuite/util/testsuite_fs.h (__gnu_test::nonexistent_path):
            Always include a counter in the filename returned.

Diff:
---
 libstdc++-v3/testsuite/util/testsuite_fs.h | 31 ++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h
index 29d0b029b75..542f9a3b983 100644
--- a/libstdc++-v3/testsuite/util/testsuite_fs.h
+++ b/libstdc++-v3/testsuite/util/testsuite_fs.h
@@ -38,9 +38,9 @@ namespace test_fs = std::experimental::filesystem;
 
 #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L
 #include <stdlib.h> // mkstemp
-#else
-#include <random>   // std::random_device
+#include <cstring> // strcpy
 #endif
+#include <random>   // std::random_device
 
 #ifndef _GLIBCXX_HAVE_SYMLINK
 #define NO_SYMLINKS
@@ -124,8 +124,32 @@ namespace __gnu_test
       file.erase(0, pos+1);
 
     test_fs::path p;
+    // A counter, starting from a random value, to be included as part
+    // of the filename being returned, and incremented each time
+    // this method is used.  It allows us to ensure that two calls
+    // to this method can never return the same filename, something
+    // testcases do when they need multiple non-existent filenames
+    // for their purposes.
+    static unsigned counter = std::random_device{}();
+
 #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L
-    char tmp[] = "filesystem-test.XXXXXX";
+    // Use mkstemp to determine the name of a file which does not exist yet.
+    //
+    // Note that we have seen on some systems (such as RTEMS, for instance)
+    // that mkstemp behaves very predictably, causing it to always try
+    // the same sequence of file names.  In other words, if we call mkstemp
+    // with a pattern, delete the file it created (which is what we do, here),
+    // and call mkstemp with the same pattern again, it returns the same
+    // filename once more.  While most implementations introduce a degree
+    // of randomness, it is not mandated by the standard, and this is why
+    // we include a counter in the template passed to mkstemp.
+    std::string mkstemp_template ("filesystem-test.");
+    mkstemp_template.append(std::to_string (counter++));
+    mkstemp_template.append(".XXXXXX");
+
+    char tmp[mkstemp_template.length() + 1];
+    std::strcpy (tmp, mkstemp_template.c_str());
+
     int fd = ::mkstemp(tmp);
     if (fd == -1)
       throw test_fs::filesystem_error("mkstemp failed",
@@ -140,7 +164,6 @@ namespace __gnu_test
     if (file.length() > 64)
       file.resize(64);
     char buf[128];
-    static unsigned counter = std::random_device{}();
 #if _GLIBCXX_USE_C99_STDIO
     std::snprintf(buf, 128,
 #else


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [gcc(refs/users/aoliva/heads/testme)] __gnu_test::nonexistent_path: Always include counter in filename returned
@ 2022-06-23 12:30 Alexandre Oliva
  0 siblings, 0 replies; 7+ messages in thread
From: Alexandre Oliva @ 2022-06-23 12:30 UTC (permalink / raw)
  To: gcc-cvs, libstdc++-cvs

https://gcc.gnu.org/g:d6496f76536ef77d034c1b9ff954659a361f5fa3

commit d6496f76536ef77d034c1b9ff954659a361f5fa3
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Thu Jun 23 05:03:54 2022 -0300

    __gnu_test::nonexistent_path: Always include counter in filename returned
    
    We have noticed that, on RTEMS, a small number of testscases are
    failing because two calls to this method return the same filename.
    This happens for instance in 27_io/filesystem/operations/copy_file.cc
    where it does:
    
      auto from = __gnu_test::nonexistent_path();
      auto to = __gnu_test::nonexistent_path();
    
    We tracked this issue down to the fact that the implementation of
    mkstemp on that system appears to use a very predictable algorithm
    for chosing the name of the temporary file, where the same filename
    appears to be tried in the same order, regardless of past calls.
    So, as long as the file gets deleted after a call to mkstemp (something
    we do here in our nonexistent_path method), the next call to mkstemps
    ends up returning the same filename, causing the collision we se above.
    
    This commit enhances the __gnu_test::nonexistent_path method to
    introduce in the filename being returned a counter which gets
    incremented at every call of this method.
    
    libstdc++-v3/ChangeLog:
    
            * testsuite/util/testsuite_fs.h (__gnu_test::nonexistent_path):
            Always include a counter in the filename returned.

Diff:
---
 libstdc++-v3/testsuite/util/testsuite_fs.h | 31 ++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h
index 29d0b029b75..542f9a3b983 100644
--- a/libstdc++-v3/testsuite/util/testsuite_fs.h
+++ b/libstdc++-v3/testsuite/util/testsuite_fs.h
@@ -38,9 +38,9 @@ namespace test_fs = std::experimental::filesystem;
 
 #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L
 #include <stdlib.h> // mkstemp
-#else
-#include <random>   // std::random_device
+#include <cstring> // strcpy
 #endif
+#include <random>   // std::random_device
 
 #ifndef _GLIBCXX_HAVE_SYMLINK
 #define NO_SYMLINKS
@@ -124,8 +124,32 @@ namespace __gnu_test
       file.erase(0, pos+1);
 
     test_fs::path p;
+    // A counter, starting from a random value, to be included as part
+    // of the filename being returned, and incremented each time
+    // this method is used.  It allows us to ensure that two calls
+    // to this method can never return the same filename, something
+    // testcases do when they need multiple non-existent filenames
+    // for their purposes.
+    static unsigned counter = std::random_device{}();
+
 #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L
-    char tmp[] = "filesystem-test.XXXXXX";
+    // Use mkstemp to determine the name of a file which does not exist yet.
+    //
+    // Note that we have seen on some systems (such as RTEMS, for instance)
+    // that mkstemp behaves very predictably, causing it to always try
+    // the same sequence of file names.  In other words, if we call mkstemp
+    // with a pattern, delete the file it created (which is what we do, here),
+    // and call mkstemp with the same pattern again, it returns the same
+    // filename once more.  While most implementations introduce a degree
+    // of randomness, it is not mandated by the standard, and this is why
+    // we include a counter in the template passed to mkstemp.
+    std::string mkstemp_template ("filesystem-test.");
+    mkstemp_template.append(std::to_string (counter++));
+    mkstemp_template.append(".XXXXXX");
+
+    char tmp[mkstemp_template.length() + 1];
+    std::strcpy (tmp, mkstemp_template.c_str());
+
     int fd = ::mkstemp(tmp);
     if (fd == -1)
       throw test_fs::filesystem_error("mkstemp failed",
@@ -140,7 +164,6 @@ namespace __gnu_test
     if (file.length() > 64)
       file.resize(64);
     char buf[128];
-    static unsigned counter = std::random_device{}();
 #if _GLIBCXX_USE_C99_STDIO
     std::snprintf(buf, 128,
 #else


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [gcc(refs/users/aoliva/heads/testme)] __gnu_test::nonexistent_path: Always include counter in filename returned
@ 2022-06-23 12:22 Alexandre Oliva
  0 siblings, 0 replies; 7+ messages in thread
From: Alexandre Oliva @ 2022-06-23 12:22 UTC (permalink / raw)
  To: gcc-cvs, libstdc++-cvs

https://gcc.gnu.org/g:ccd41a65c88c5f4ca4ad0135ad44a4782f40e1c2

commit ccd41a65c88c5f4ca4ad0135ad44a4782f40e1c2
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Thu Jun 23 05:03:54 2022 -0300

    __gnu_test::nonexistent_path: Always include counter in filename returned
    
    We have noticed that, on RTEMS, a small number of testscases are
    failing because two calls to this method return the same filename.
    This happens for instance in 27_io/filesystem/operations/copy_file.cc
    where it does:
    
      auto from = __gnu_test::nonexistent_path();
      auto to = __gnu_test::nonexistent_path();
    
    We tracked this issue down to the fact that the implementation of
    mkstemp on that system appears to use a very predictable algorithm
    for chosing the name of the temporary file, where the same filename
    appears to be tried in the same order, regardless of past calls.
    So, as long as the file gets deleted after a call to mkstemp (something
    we do here in our nonexistent_path method), the next call to mkstemps
    ends up returning the same filename, causing the collision we se above.
    
    This commit enhances the __gnu_test::nonexistent_path method to
    introduce in the filename being returned a counter which gets
    incremented at every call of this method.
    
    libstdc++-v3/ChangeLog:
    
            * testsuite/util/testsuite_fs.h (__gnu_test::nonexistent_path):
            Always include a counter in the filename returned.

Diff:
---
 libstdc++-v3/testsuite/util/testsuite_fs.h | 31 ++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h
index 037d9ffc0f4..206ea677790 100644
--- a/libstdc++-v3/testsuite/util/testsuite_fs.h
+++ b/libstdc++-v3/testsuite/util/testsuite_fs.h
@@ -38,9 +38,9 @@ namespace test_fs = std::experimental::filesystem;
 
 #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L
 #include <stdlib.h> // mkstemp
-#else
-#include <random>   // std::random_device
+#include <cstring> // strcpy
 #endif
+#include <random>   // std::random_device
 
 #if defined(__MINGW32__) || defined(__MINGW64__) \
   || !defined (_GLIBCXX_HAVE_SYMLINK)
@@ -125,8 +125,32 @@ namespace __gnu_test
       file.erase(0, pos+1);
 
     test_fs::path p;
+    // A counter, starting from a random value, to be included as part
+    // of the filename being returned, and incremented each time
+    // this method is used.  It allows us to ensure that two calls
+    // to this method can never return the same filename, something
+    // testcases do when they need multiple non-existent filenames
+    // for their purposes.
+    static unsigned counter = std::random_device{}();
+
 #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L
-    char tmp[] = "filesystem-test.XXXXXX";
+    // Use mkstemp to determine the name of a file which does not exist yet.
+    //
+    // Note that we have seen on some systems (such as RTEMS, for instance)
+    // that mkstemp behaves very predictably, causing it to always try
+    // the same sequence of file names.  In other words, if we call mkstemp
+    // with a pattern, delete the file it created (which is what we do, here),
+    // and call mkstemp with the same pattern again, it returns the same
+    // filename once more.  While most implementations introduce a degree
+    // of randomness, it is not mandated by the standard, and this is why
+    // we include a counter in the template passed to mkstemp.
+    std::string mkstemp_template ("filesystem-test.");
+    mkstemp_template.append(std::to_string (counter++));
+    mkstemp_template.append(".XXXXXX");
+
+    char tmp[mkstemp_template.length() + 1];
+    std::strcpy (tmp, mkstemp_template.c_str());
+
     int fd = ::mkstemp(tmp);
     if (fd == -1)
       throw test_fs::filesystem_error("mkstemp failed",
@@ -141,7 +165,6 @@ namespace __gnu_test
     if (file.length() > 64)
       file.resize(64);
     char buf[128];
-    static unsigned counter = std::random_device{}();
 #if _GLIBCXX_USE_C99_STDIO
     std::snprintf(buf, 128,
 #else


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [gcc(refs/users/aoliva/heads/testme)] __gnu_test::nonexistent_path: Always include counter in filename returned
@ 2022-06-23 10:05 Alexandre Oliva
  0 siblings, 0 replies; 7+ messages in thread
From: Alexandre Oliva @ 2022-06-23 10:05 UTC (permalink / raw)
  To: gcc-cvs, libstdc++-cvs

https://gcc.gnu.org/g:f95af16ff5dd351dce665f886a3b6f3554f4b2b0

commit f95af16ff5dd351dce665f886a3b6f3554f4b2b0
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Thu Jun 23 05:03:54 2022 -0300

    __gnu_test::nonexistent_path: Always include counter in filename returned
    
    We have noticed that, on RTEMS, a small number of testscases are
    failing because two calls to this method return the same filename.
    This happens for instance in 27_io/filesystem/operations/copy_file.cc
    where it does:
    
      auto from = __gnu_test::nonexistent_path();
      auto to = __gnu_test::nonexistent_path();
    
    We tracked this issue down to the fact that the implementation of
    mkstemp on that system appears to use a very predictable algorithm
    for chosing the name of the temporary file, where the same filename
    appears to be tried in the same order, regardless of past calls.
    So, as long as the file gets deleted after a call to mkstemp (something
    we do here in our nonexistent_path method), the next call to mkstemps
    ends up returning the same filename, causing the collision we se above.
    
    This commit enhances the __gnu_test::nonexistent_path method to
    introduce in the filename being returned a counter which gets
    incremented at every call of this method.
    
    libstdc++-v3/ChangeLog:
    
            * testsuite/util/testsuite_fs.h (__gnu_test::nonexistent_path):
            Always include a counter in the filename returned.

Diff:
---
 libstdc++-v3/testsuite/util/testsuite_fs.h | 31 ++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h
index 037d9ffc0f4..206ea677790 100644
--- a/libstdc++-v3/testsuite/util/testsuite_fs.h
+++ b/libstdc++-v3/testsuite/util/testsuite_fs.h
@@ -38,9 +38,9 @@ namespace test_fs = std::experimental::filesystem;
 
 #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L
 #include <stdlib.h> // mkstemp
-#else
-#include <random>   // std::random_device
+#include <cstring> // strcpy
 #endif
+#include <random>   // std::random_device
 
 #if defined(__MINGW32__) || defined(__MINGW64__) \
   || !defined (_GLIBCXX_HAVE_SYMLINK)
@@ -125,8 +125,32 @@ namespace __gnu_test
       file.erase(0, pos+1);
 
     test_fs::path p;
+    // A counter, starting from a random value, to be included as part
+    // of the filename being returned, and incremented each time
+    // this method is used.  It allows us to ensure that two calls
+    // to this method can never return the same filename, something
+    // testcases do when they need multiple non-existent filenames
+    // for their purposes.
+    static unsigned counter = std::random_device{}();
+
 #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L
-    char tmp[] = "filesystem-test.XXXXXX";
+    // Use mkstemp to determine the name of a file which does not exist yet.
+    //
+    // Note that we have seen on some systems (such as RTEMS, for instance)
+    // that mkstemp behaves very predictably, causing it to always try
+    // the same sequence of file names.  In other words, if we call mkstemp
+    // with a pattern, delete the file it created (which is what we do, here),
+    // and call mkstemp with the same pattern again, it returns the same
+    // filename once more.  While most implementations introduce a degree
+    // of randomness, it is not mandated by the standard, and this is why
+    // we include a counter in the template passed to mkstemp.
+    std::string mkstemp_template ("filesystem-test.");
+    mkstemp_template.append(std::to_string (counter++));
+    mkstemp_template.append(".XXXXXX");
+
+    char tmp[mkstemp_template.length() + 1];
+    std::strcpy (tmp, mkstemp_template.c_str());
+
     int fd = ::mkstemp(tmp);
     if (fd == -1)
       throw test_fs::filesystem_error("mkstemp failed",
@@ -141,7 +165,6 @@ namespace __gnu_test
     if (file.length() > 64)
       file.resize(64);
     char buf[128];
-    static unsigned counter = std::random_device{}();
 #if _GLIBCXX_USE_C99_STDIO
     std::snprintf(buf, 128,
 #else


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [gcc(refs/users/aoliva/heads/testme)] __gnu_test::nonexistent_path: Always include counter in filename returned
@ 2022-06-23  8:40 Alexandre Oliva
  0 siblings, 0 replies; 7+ messages in thread
From: Alexandre Oliva @ 2022-06-23  8:40 UTC (permalink / raw)
  To: gcc-cvs, libstdc++-cvs

https://gcc.gnu.org/g:7bb71fb886efdf2c086839b254964e1fccfb1dc6

commit 7bb71fb886efdf2c086839b254964e1fccfb1dc6
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Thu Jun 23 05:03:54 2022 -0300

    __gnu_test::nonexistent_path: Always include counter in filename returned
    
    We have noticed that, on RTEMS, a small number of testscases are
    failing because two calls to this method return the same filename.
    This happens for instance in 27_io/filesystem/operations/copy_file.cc
    where it does:
    
      auto from = __gnu_test::nonexistent_path();
      auto to = __gnu_test::nonexistent_path();
    
    We tracked this issue down to the fact that the implementation of
    mkstemp on that system appears to use a very predictable algorithm
    for chosing the name of the temporary file, where the same filename
    appears to be tried in the same order, regardless of past calls.
    So, as long as the file gets deleted after a call to mkstemp (something
    we do here in our nonexistent_path method), the next call to mkstemps
    ends up returning the same filename, causing the collision we se above.
    
    This commit enhances the __gnu_test::nonexistent_path method to
    introduce in the filename being returned a counter which gets
    incremented at every call of this method.
    
    libstdc++-v3/ChangeLog:
    
            * testsuite/util/testsuite_fs.h (__gnu_test::nonexistent_path):
            Always include a counter in the filename returned.

Diff:
---
 libstdc++-v3/testsuite/util/testsuite_fs.h | 31 ++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h
index 037d9ffc0f4..206ea677790 100644
--- a/libstdc++-v3/testsuite/util/testsuite_fs.h
+++ b/libstdc++-v3/testsuite/util/testsuite_fs.h
@@ -38,9 +38,9 @@ namespace test_fs = std::experimental::filesystem;
 
 #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L
 #include <stdlib.h> // mkstemp
-#else
-#include <random>   // std::random_device
+#include <cstring> // strcpy
 #endif
+#include <random>   // std::random_device
 
 #if defined(__MINGW32__) || defined(__MINGW64__) \
   || !defined (_GLIBCXX_HAVE_SYMLINK)
@@ -125,8 +125,32 @@ namespace __gnu_test
       file.erase(0, pos+1);
 
     test_fs::path p;
+    // A counter, starting from a random value, to be included as part
+    // of the filename being returned, and incremented each time
+    // this method is used.  It allows us to ensure that two calls
+    // to this method can never return the same filename, something
+    // testcases do when they need multiple non-existent filenames
+    // for their purposes.
+    static unsigned counter = std::random_device{}();
+
 #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L
-    char tmp[] = "filesystem-test.XXXXXX";
+    // Use mkstemp to determine the name of a file which does not exist yet.
+    //
+    // Note that we have seen on some systems (such as RTEMS, for instance)
+    // that mkstemp behaves very predictably, causing it to always try
+    // the same sequence of file names.  In other words, if we call mkstemp
+    // with a pattern, delete the file it created (which is what we do, here),
+    // and call mkstemp with the same pattern again, it returns the same
+    // filename once more.  While most implementations introduce a degree
+    // of randomness, it is not mandated by the standard, and this is why
+    // we include a counter in the template passed to mkstemp.
+    std::string mkstemp_template ("filesystem-test.");
+    mkstemp_template.append(std::to_string (counter++));
+    mkstemp_template.append(".XXXXXX");
+
+    char tmp[mkstemp_template.length() + 1];
+    std::strcpy (tmp, mkstemp_template.c_str());
+
     int fd = ::mkstemp(tmp);
     if (fd == -1)
       throw test_fs::filesystem_error("mkstemp failed",
@@ -141,7 +165,6 @@ namespace __gnu_test
     if (file.length() > 64)
       file.resize(64);
     char buf[128];
-    static unsigned counter = std::random_device{}();
 #if _GLIBCXX_USE_C99_STDIO
     std::snprintf(buf, 128,
 #else


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [gcc(refs/users/aoliva/heads/testme)] __gnu_test::nonexistent_path: Always include counter in filename returned
@ 2022-06-23  8:07 Alexandre Oliva
  0 siblings, 0 replies; 7+ messages in thread
From: Alexandre Oliva @ 2022-06-23  8:07 UTC (permalink / raw)
  To: gcc-cvs, libstdc++-cvs

https://gcc.gnu.org/g:0775674e1163f079712dc24b39f0016bd9591d49

commit 0775674e1163f079712dc24b39f0016bd9591d49
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Thu Jun 23 05:03:54 2022 -0300

    __gnu_test::nonexistent_path: Always include counter in filename returned
    
    We have noticed that, on RTEMS, a small number of testscases are
    failing because two calls to this method return the same filename.
    This happens for instance in 27_io/filesystem/operations/copy_file.cc
    where it does:
    
      auto from = __gnu_test::nonexistent_path();
      auto to = __gnu_test::nonexistent_path();
    
    We tracked this issue down to the fact that the implementation of
    mkstemp on that system appears to use a very predictable algorithm
    for chosing the name of the temporary file, where the same filename
    appears to be tried in the same order, regardless of past calls.
    So, as long as the file gets deleted after a call to mkstemp (something
    we do here in our nonexistent_path method), the next call to mkstemps
    ends up returning the same filename, causing the collision we se above.
    
    This commit enhances the __gnu_test::nonexistent_path method to
    introduce in the filename being returned a counter which gets
    incremented at every call of this method.
    
    libstdc++-v3/ChangeLog:
    
            * testsuite/util/testsuite_fs.h (__gnu_test::nonexistent_path):
            Always include a counter in the filename returned.

Diff:
---
 libstdc++-v3/testsuite/util/testsuite_fs.h | 31 ++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h
index 037d9ffc0f4..206ea677790 100644
--- a/libstdc++-v3/testsuite/util/testsuite_fs.h
+++ b/libstdc++-v3/testsuite/util/testsuite_fs.h
@@ -38,9 +38,9 @@ namespace test_fs = std::experimental::filesystem;
 
 #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L
 #include <stdlib.h> // mkstemp
-#else
-#include <random>   // std::random_device
+#include <cstring> // strcpy
 #endif
+#include <random>   // std::random_device
 
 #if defined(__MINGW32__) || defined(__MINGW64__) \
   || !defined (_GLIBCXX_HAVE_SYMLINK)
@@ -125,8 +125,32 @@ namespace __gnu_test
       file.erase(0, pos+1);
 
     test_fs::path p;
+    // A counter, starting from a random value, to be included as part
+    // of the filename being returned, and incremented each time
+    // this method is used.  It allows us to ensure that two calls
+    // to this method can never return the same filename, something
+    // testcases do when they need multiple non-existent filenames
+    // for their purposes.
+    static unsigned counter = std::random_device{}();
+
 #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L
-    char tmp[] = "filesystem-test.XXXXXX";
+    // Use mkstemp to determine the name of a file which does not exist yet.
+    //
+    // Note that we have seen on some systems (such as RTEMS, for instance)
+    // that mkstemp behaves very predictably, causing it to always try
+    // the same sequence of file names.  In other words, if we call mkstemp
+    // with a pattern, delete the file it created (which is what we do, here),
+    // and call mkstemp with the same pattern again, it returns the same
+    // filename once more.  While most implementations introduce a degree
+    // of randomness, it is not mandated by the standard, and this is why
+    // we include a counter in the template passed to mkstemp.
+    std::string mkstemp_template ("filesystem-test.");
+    mkstemp_template.append(std::to_string (counter++));
+    mkstemp_template.append(".XXXXXX");
+
+    char tmp[mkstemp_template.length() + 1];
+    std::strcpy (tmp, mkstemp_template.c_str());
+
     int fd = ::mkstemp(tmp);
     if (fd == -1)
       throw test_fs::filesystem_error("mkstemp failed",
@@ -141,7 +165,6 @@ namespace __gnu_test
     if (file.length() > 64)
       file.resize(64);
     char buf[128];
-    static unsigned counter = std::random_device{}();
 #if _GLIBCXX_USE_C99_STDIO
     std::snprintf(buf, 128,
 #else


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-06-23 12:44 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-23  9:03 [gcc(refs/users/aoliva/heads/testme)] __gnu_test::nonexistent_path: Always include counter in filename returned Alexandre Oliva
  -- strict thread matches above, loose matches on Subject: below --
2022-06-23 12:44 Alexandre Oliva
2022-06-23 12:30 Alexandre Oliva
2022-06-23 12:22 Alexandre Oliva
2022-06-23 10:05 Alexandre Oliva
2022-06-23  8:40 Alexandre Oliva
2022-06-23  8:07 Alexandre Oliva

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).