Skip to content

Commit

Permalink
Remove calls to tmpnam() in protobuf/testing/googletest.cc.
Browse files Browse the repository at this point in the history
This came up as a pull request here:
#18239.

|tmpnam| is ugly and would be nice to avoid.

We don't necessarily want to substitute out GetTemporaryDirectoryName with
|std::filesystem::temp_directory_path| -- the current setup creates a
subdirectory under the tmp directory and uses TempDirDeleter to clean it up.
We want to preserve that behaviour: the current setup plays nicely with bazel
without polluting /tmp with unbounded growth.

#test-continuous

PiperOrigin-RevId: 677843997
  • Loading branch information
tonyliaoss authored and copybara-github committed Sep 23, 2024
1 parent 6f5b35b commit 86874ce
Showing 1 changed file with 3 additions and 38 deletions.
41 changes: 3 additions & 38 deletions src/google/protobuf/testing/googletest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
#include "absl/strings/str_replace.h"
#include "google/protobuf/io/io_win32.h"
#include "google/protobuf/testing/file.h"
#include <gtest/gtest.h>

#ifdef _MSC_VER
// #include <direct.h>
#else
Expand Down Expand Up @@ -93,45 +95,8 @@ std::string TestSourceDir() {
namespace {

std::string GetTemporaryDirectoryName() {
// Tests run under Bazel "should not" use /tmp. Bazel sets this environment
// variable for tests to use instead.
char* from_environment = getenv("TEST_TMPDIR");
if (from_environment != nullptr && from_environment[0] != '\0') {
return absl::StrCat(from_environment, "/protobuf_tmpdir");
}

// tmpnam() is generally not considered safe but we're only using it for
// testing. We cannot use tmpfile() or mkstemp() since we're creating a
// directory.
char b[L_tmpnam + 1]; // HPUX multithread return 0 if s is 0
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
std::string result = tmpnam(b);
#pragma GCC diagnostic pop
std::string result = absl::StrCat(testing::TempDir(), "protobuf_tempdir");
#ifdef _WIN32
// Avoid a trailing dot by changing it to an underscore. On Win32 the names of
// files and directories can, but should not, end with dot.
//
// In MS-DOS and FAT16 filesystem the filenames were 8dot3 style so it didn't
// make sense to have a name ending in dot without an extension, so the shell
// silently ignored trailing dots. To this day the Win32 API still maintains
// this behavior and silently ignores trailing dots in path arguments of
// functions such as CreateFile{A,W}. Even POSIX API function implementations
// seem to wrap the Win32 API functions (e.g. CreateDirectoryA) and behave
// this way.
// It's possible to avoid this behavior and create files / directories with
// trailing dots (using CreateFileW / CreateDirectoryW and prefixing the path
// with "\\?\") but these will be degenerate in the sense that you cannot
// chdir into such directories (or navigate into them with Windows Explorer)
// nor can you open such files with some programs (e.g. Notepad).
if (result[result.size() - 1] == '.') {
result[result.size() - 1] = '_';
}
// On Win32, tmpnam() returns a file prefixed with '\', but which is supposed
// to be used in the current working directory. WTF?
if (absl::StartsWith(result, "\\")) {
result.erase(0, 1);
}
// The Win32 API accepts forward slashes as a path delimiter as long as the
// path doesn't use the "\\?\" prefix.
// Let's avoid confusion and use only forward slashes.
Expand Down

0 comments on commit 86874ce

Please sign in to comment.