[PATCH] libstdc++: testsuite: avoid predictable mkstemp

[PATCH] libstdc++: testsuite: avoid predictable mkstemp

[Alexandre Oliva]

This patch was originally meant to reduce the likelihood that
nonexistent_path() returns the same pathname for from and to.

It was prompted by a target system with a non-random implementation of
mkstemp, that returns a predictable sequence of filenames and selects
the first one that isn't already taken.

That turned out not to be enough: nonexistent_path adds a suffix to
the filename chosen by mkstemp and removes the file it created, so
mkstemp may very well insist on the same basename, and the case that
doesn't use mkstemp doesn't even check whether the file already
exists.

Anyway, by the time I realized this wasn't enough, I'd already
implemented some of the changes, and I figured I might as well
contribute them, even though they don't really solve any problem, and
even if they did, they'd be just a partial solution.

Regstrapped on x86_64-linux-gnu, also tested with a cross to
aarch64-rtems6.  Ok to install?


for  libstdc++-v3/ChangeLog

	* testsuite/27_io/filesystem/operations/copy.cc (test02):
	Select TO after creating FROM.
	(test03, test04): Likewise.
	* testsuite/experimental/filesystem/operations/copy.cc
	(test02, test03, test04): Likewise.
---
 .../testsuite/27_io/filesystem/operations/copy.cc  |    7 ++++---
 .../experimental/filesystem/operations/copy.cc     |    7 ++++---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/copy.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/copy.cc
index b936e04493b5c..f3081f4b64ebc 100644
--- a/libstdc++-v3/testsuite/27_io/filesystem/operations/copy.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/copy.cc
@@ -73,7 +73,6 @@ test02()
 
   const std::error_code bad_ec = make_error_code(std::errc::invalid_argument);
   auto from = __gnu_test::nonexistent_path();
-  auto to = __gnu_test::nonexistent_path();
   std::error_code ec;
 
   ec = bad_ec;
@@ -81,6 +80,7 @@ test02()
   VERIFY( !ec );
   VERIFY( fs::exists(from) );
 
+  auto to = __gnu_test::nonexistent_path();
   ec = bad_ec;
   fs::copy(from, to, fs::copy_options::skip_symlinks, ec);
   VERIFY( !ec );
@@ -117,12 +117,13 @@ void
 test03()
 {
   auto from = __gnu_test::nonexistent_path();
-  auto to = __gnu_test::nonexistent_path();
 
   // test empty file
   std::ofstream{from};
   VERIFY( fs::exists(from) );
   VERIFY( fs::file_size(from) == 0 );
+
+  auto to = __gnu_test::nonexistent_path();
   fs::copy(from, to);
   VERIFY( fs::exists(to) );
   VERIFY( fs::file_size(to) == 0 );
@@ -145,11 +146,11 @@ test04()
 {
   const std::error_code bad_ec = make_error_code(std::errc::invalid_argument);
   auto from = __gnu_test::nonexistent_path();
-  auto to = __gnu_test::nonexistent_path();
   std::error_code ec;
 
   create_directories(from/"a/b/c");
 
+  auto to = __gnu_test::nonexistent_path();
   {
     __gnu_test::scoped_file f(to);
     copy(from, to, ec);
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc
index 5cd6b483c269b..ca38328c5da15 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc
@@ -73,7 +73,6 @@ test02()
 #endif
 
   auto from = __gnu_test::nonexistent_path();
-  auto to = __gnu_test::nonexistent_path();
   std::error_code ec, bad = std::make_error_code(std::errc::invalid_argument);
 
   ec = bad;
@@ -81,6 +80,7 @@ test02()
   VERIFY( !ec );
   VERIFY( fs::exists(from) );
 
+  auto to = __gnu_test::nonexistent_path();
   ec = bad;
   fs::copy(from, to, fs::copy_options::skip_symlinks, ec);
   VERIFY( !ec );
@@ -116,12 +116,13 @@ void
 test03()
 {
   auto from = __gnu_test::nonexistent_path();
-  auto to = __gnu_test::nonexistent_path();
 
   // test empty file
   std::ofstream{from.c_str()};
   VERIFY( fs::exists(from) );
   VERIFY( fs::file_size(from) == 0 );
+
+  auto to = __gnu_test::nonexistent_path();
   fs::copy(from, to);
   VERIFY( fs::exists(to) );
   VERIFY( fs::file_size(to) == 0 );
@@ -143,11 +144,11 @@ void
 test04()
 {
   auto from = __gnu_test::nonexistent_path();
-  auto to = __gnu_test::nonexistent_path();
   std::error_code ec;
 
   create_directories(from/"a/b/c");
 
+  auto to = __gnu_test::nonexistent_path();
   {
     __gnu_test::scoped_file f(to);
     copy(from, to, ec);

-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

Re: [PATCH] libstdc++: testsuite: avoid predictable mkstemp

[Jonathan Wakely]

On Wed, 22 Jun 2022 at 07:05, Alexandre Oliva via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
>
> This patch was originally meant to reduce the likelihood that
> nonexistent_path() returns the same pathname for from and to.
>
> It was prompted by a target system with a non-random implementation of
> mkstemp, that returns a predictable sequence of filenames and selects
> the first one that isn't already taken.
>
> That turned out not to be enough: nonexistent_path adds a suffix to
> the filename chosen by mkstemp and removes the file it created, so
> mkstemp may very well insist on the same basename, and the case that
> doesn't use mkstemp doesn't even check whether the file already
> exists.
>
> Anyway, by the time I realized this wasn't enough, I'd already
> implemented some of the changes, and I figured I might as well
> contribute them, even though they don't really solve any problem, and
> even if they did, they'd be just a partial solution.
>
> Regstrapped on x86_64-linux-gnu, also tested with a cross to
> aarch64-rtems6.  Ok to install?

OK

Re: [PATCH] libstdc++: testsuite: avoid predictable mkstemp

[Alexandre Oliva]

On Jun 22, 2022, Jonathan Wakely <jwakely@redhat.com> wrote:

> On Wed, 22 Jun 2022 at 07:05, Alexandre Oliva via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:

>> It was prompted by a target system with a non-random implementation of
>> mkstemp, that returns a predictable sequence of filenames and selects
>> the first one that isn't already taken.

> OK

And here's the patch that enabled me to stop worrying about the above.
Regstrapped on x86_64-linux-gnu, also tested with a cross to
aarch64-rtems6.  Ok to install?


__gnu_test::nonexistent_path: Always include counter in filename returned

From: Joel Brobecker <brobecker@adacore.com>

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.
---
 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 037d9ffc0f429..206ea67779003 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


-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

Re: [PATCH] libstdc++: testsuite: avoid predictable mkstemp

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 4690 bytes --]

On Thu, 23 Jun 2022 at 12:39, Alexandre Oliva <oliva@adacore.com> wrote:
>
> On Jun 22, 2022, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> > On Wed, 22 Jun 2022 at 07:05, Alexandre Oliva via Libstdc++
> > <libstdc++@gcc.gnu.org> wrote:
>
> >> It was prompted by a target system with a non-random implementation of
> >> mkstemp, that returns a predictable sequence of filenames and selects
> >> the first one that isn't already taken.
>
> > OK
>
> And here's the patch that enabled me to stop worrying about the above.
> Regstrapped on x86_64-linux-gnu, also tested with a cross to
> aarch64-rtems6.  Ok to install?
>
>
> __gnu_test::nonexistent_path: Always include counter in filename returned
>
> From: Joel Brobecker <brobecker@adacore.com>
>
> 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.
> ---
>  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 037d9ffc0f429..206ea67779003 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());

std::string::copy can be used to copy a string into a buffer, but
there's no reason to use another buffer anyway.

The attached makes this a bit more efficient, and makes more of the
code common to the mkstemp and non-mkstmp branches. I'll wait to hear
back from you before pushing it (since it has Joel's name on the
patch).

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 4808 bytes --]

commit 833337d861e3126f6304e8bc9d8e743022a21424
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Thu Jun 23 13:12:12 2022

    libstdc++: testsuite: avoid predicable mkstemp
    
    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.
    
    Co-authored-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            * testsuite/util/testsuite_fs.h (__gnu_test::nonexistent_path):
            Always include a counter in the filename returned.

diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h
index 9358a04e56c..3a197c70a1e 100644
--- a/libstdc++-v3/testsuite/util/testsuite_fs.h
+++ b/libstdc++-v3/testsuite/util/testsuite_fs.h
@@ -32,14 +32,14 @@ namespace test_fs = std::experimental::filesystem;
 #endif
 #include <algorithm>
 #include <fstream>
+#include <random>   // std::random_device
 #include <string>
+#include <system_error>
 #include <cstdio>
 #include <unistd.h> // unlink, close, getpid, geteuid
 
 #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L
 #include <stdlib.h> // mkstemp
-#else
-#include <random>   // std::random_device
 #endif
 
 namespace __gnu_test
@@ -109,32 +109,51 @@ namespace __gnu_test
     if (pos != file.npos)
       file.erase(0, pos+1);
 
+    file.reserve(file.size() + 40);
+    file.insert(0, "filesystem-test.");
+
+    // A counter, starting from a random value, to be included as part
+    // of the filename being returned, and incremented each time
+    // this function is used.  It allows us to ensure that two calls
+    // to this function 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{}();
+    file += '.';
+    file += std::to_string(counter++);
+    file += '.';
+
     test_fs::path p;
 #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L
-    char tmp[] = "filesystem-test.XXXXXX";
-    int fd = ::mkstemp(tmp);
+
+    // 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 also include a counter in the template passed to mkstemp.
+    file += "XXXXXX";
+    int fd = ::mkstemp(&file[0]);
     if (fd == -1)
       throw test_fs::filesystem_error("mkstemp failed",
 	  std::error_code(errno, std::generic_category()));
-    ::unlink(tmp);
+    ::unlink(file.c_str());
     ::close(fd);
-    if (!file.empty())
-      file.insert(0, 1, '-');
-    file.insert(0, tmp);
-    p = file;
+    p = std::move(file);
 #else
     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
-    std::sprintf(buf,
-#endif
-      "filesystem-test.%u.%lu-%s", counter++, (unsigned long) ::getpid(),
-      file.c_str());
-    p = buf;
+    // The combination of random counter and PID should be unique for a given
+    // run of the testsuite.
+    file += std::to_string(::getpid());
+    p = std::move(file);
+    if (test_fs::exists(p))
+      throw test_fs::filesystem_error("Failed to generate unique pathname", p,
+	  std::make_error_code(std::errc::file_exists));
 #endif
     return p;
   }

Re: [PATCH] libstdc++: testsuite: avoid predictable mkstemp

[Alexandre Oliva]

On Jun 23, 2022, Jonathan Wakely <jwakely@redhat.com> wrote:

> The attached makes this a bit more efficient, and makes more of the
> code common to the mkstemp and non-mkstmp branches. I'll wait to hear
> back from you before pushing it (since it has Joel's name on the
> patch).

Thanks, I've given it a spin, both trunk and gcc-11, and I confirm it
works for us.

-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

Re: [PATCH] libstdc++: testsuite: avoid predictable mkstemp

[Jonathan Wakely]

On Mon, 27 Jun 2022 at 10:32, Alexandre Oliva <oliva@adacore.com> wrote:
>
> On Jun 23, 2022, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> > The attached makes this a bit more efficient, and makes more of the
> > code common to the mkstemp and non-mkstmp branches. I'll wait to hear
> > back from you before pushing it (since it has Joel's name on the
> > patch).
>
> Thanks, I've given it a spin, both trunk and gcc-11, and I confirm it
> works for us.


I've pushed it to trunk now.

Re: [PATCH] libstdc++: testsuite: avoid predictable mkstemp

[Alexandre Oliva]

On Jun 27, 2022, Alexandre Oliva <oliva@adacore.com> wrote:

> On Jun 23, 2022, Jonathan Wakely <jwakely@redhat.com> wrote:
>> The attached makes this a bit more efficient, and makes more of the
>> code common to the mkstemp and non-mkstmp branches. I'll wait to hear
>> back from you before pushing it (since it has Joel's name on the
>> patch).

> Thanks, I've given it a spin, both trunk and gcc-11, and I confirm it
> works for us.

The bad news is that it broke on some other systems I didn't test back
then.  It turns out the type cast for the ::getpid result was not just
because it was passed to printf before :-/


libstdc++: testsuite: cast getpid result

On vxworks, in kernel mode, getpid's return type is a pointer type, so
std::to_string on it fails overload resolution.  Restore the type cast
from the original patch that suggested adding the pid.

Regstrapped on x86_64-linux-gnu, also tested on aarch64-rtems6.0 and
ppc64-vx7r2.  I'm going ahead and checking this in as obvious.  Please
let me know if you'd prefer this to be fixed in a different way.


for  libstdc++/ChangeLog

	* testsuite/util/testsuite_fs.h (nonexistent_path): Convert
	the getpid result to an integral type.
---
 libstdc++-v3/testsuite/util/testsuite_fs.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h
index 908fcdbcaeed1..25f8f734dc792 100644
--- a/libstdc++-v3/testsuite/util/testsuite_fs.h
+++ b/libstdc++-v3/testsuite/util/testsuite_fs.h
@@ -163,7 +163,7 @@ namespace __gnu_test
       file.resize(64);
     // The combination of random counter and PID should be unique for a given
     // run of the testsuite.
-    file += std::to_string(::getpid());
+    file += std::to_string((unsigned long) ::getpid());
     p = std::move(file);
     if (test_fs::exists(p))
       throw test_fs::filesystem_error("Failed to generate unique pathname", p,


-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

Re: [PATCH] libstdc++: testsuite: avoid predictable mkstemp

[Jonathan Wakely]

On Tue, 5 Jul 2022 at 10:10, Alexandre Oliva via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> On Jun 27, 2022, Alexandre Oliva <oliva@adacore.com> wrote:
>
> > On Jun 23, 2022, Jonathan Wakely <jwakely@redhat.com> wrote:
> >> The attached makes this a bit more efficient, and makes more of the
> >> code common to the mkstemp and non-mkstmp branches. I'll wait to hear
> >> back from you before pushing it (since it has Joel's name on the
> >> patch).
>
> > Thanks, I've given it a spin, both trunk and gcc-11, and I confirm it
> > works for us.
>
> The bad news is that it broke on some other systems I didn't test back
> then.  It turns out the type cast for the ::getpid result was not just
> because it was passed to printf before :-/

Ah, whoops.

I did check that POSIX requires pid_t to be a signed integer type, but
that doesn't mean it's always true for all GCC targets.

>
>
> libstdc++: testsuite: cast getpid result
>
> On vxworks, in kernel mode, getpid's return type is a pointer type, so
> std::to_string on it fails overload resolution.  Restore the type cast
> from the original patch that suggested adding the pid.
>
> Regstrapped on x86_64-linux-gnu, also tested on aarch64-rtems6.0 and
> ppc64-vx7r2.  I'm going ahead and checking this in as obvious.  Please
> let me know if you'd prefer this to be fixed in a different way.

The cast itself is fine, but I'd like a comment like "N.B. pid_t is a
pointer on vxworks" so I don't "simplify" it again.

Thanks.

Re: [PATCH] libstdc++: testsuite: avoid predictable mkstemp

[Alexandre Oliva]

On Jul  5, 2022, Jonathan Wakely <jwakely@redhat.com> wrote:

> The cast itself is fine, but I'd like a comment like "N.B. pid_t is a
> pointer on vxworks" so I don't "simplify" it again.


libstdc++: testsuite: why cast getpid result

Add a comment next to the getpid call to explain why the typecast is
needed.

Regstrapped on x86_64-linux-gnu, will install later today if there's no
objection.


for  libstdc++-v3/ChangeLog

	* testsuite/util/testsuite_fs (nonexistent_path): Explain why
	we need the typecast.
---
 libstdc++-v3/testsuite/util/testsuite_fs.h |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h
index 25f8f734dc792..0e28385e99aca 100644
--- a/libstdc++-v3/testsuite/util/testsuite_fs.h
+++ b/libstdc++-v3/testsuite/util/testsuite_fs.h
@@ -162,7 +162,8 @@ namespace __gnu_test
     if (file.length() > 64)
       file.resize(64);
     // The combination of random counter and PID should be unique for a given
-    // run of the testsuite.
+    // run of the testsuite.  N.B. getpid() returns a pointer type on vxworks
+    // in kernel mode.
     file += std::to_string((unsigned long) ::getpid());
     p = std::move(file);
     if (test_fs::exists(p))


-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

Re: [PATCH] libstdc++: testsuite: avoid predictable mkstemp

[Jonathan Wakely]

On Tue, 5 Jul 2022 at 18:46, Alexandre Oliva <oliva@adacore.com> wrote:
>
> On Jul  5, 2022, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> > The cast itself is fine, but I'd like a comment like "N.B. pid_t is a
> > pointer on vxworks" so I don't "simplify" it again.
>
>
> libstdc++: testsuite: why cast getpid result
>
> Add a comment next to the getpid call to explain why the typecast is
> needed.
>
> Regstrapped on x86_64-linux-gnu, will install later today if there's no
> objection.


Thanks.