Revert "HDFS-15971. Make mkstemp cross platform (#2898)"

This reverts commit b088f46b68.
This commit is contained in:
Eric Badger 2021-04-16 17:15:33 +00:00
parent 14816be0b1
commit aed13f0f42
13 changed files with 41 additions and 199 deletions

View File

@ -91,8 +91,7 @@ function(build_libhdfs_test NAME LIBRARY)
list(APPEND FILES ${CMAKE_SOURCE_DIR}/main/native/libhdfs-tests/${FIL})
endif()
endforeach()
add_executable("${NAME}_${LIBRARY}" $<TARGET_OBJECTS:x_platform_obj_c_api> $<TARGET_OBJECTS:x_platform_obj> ${FILES})
target_include_directories("${NAME}_${LIBRARY}" PRIVATE main/native/libhdfspp/lib)
add_executable("${NAME}_${LIBRARY}" ${FILES})
endfunction()
function(add_libhdfs_test NAME LIBRARY)

View File

@ -22,7 +22,6 @@
#include "hdfspp/hdfs_ext.h"
#include "native_mini_dfs.h"
#include "os/thread.h"
#include "x-platform/c-api/syscall.h"
#include <errno.h>
#include <inttypes.h>
@ -127,8 +126,7 @@ static int hdfsCurlData(const char *host, const tPort port, const char *dirNm,
EXPECT_NONNULL(pw = getpwuid(uid));
int fd = -1;
EXPECT_NONNEGATIVE(fd = x_platform_syscall_create_and_open_temp_file(
tmpFile, sizeof tmpFile));
EXPECT_NONNEGATIVE(fd = mkstemp(tmpFile));
tSize sz = 0;
while (sz < fileSz) {

View File

@ -18,26 +18,6 @@
#include "x-platform/syscall.h"
#include <algorithm>
#include <vector>
extern "C" {
int x_platform_syscall_write_to_stdout(const char* msg) {
extern "C" int x_platform_syscall_write_to_stdout(const char* msg) {
return XPlatform::Syscall::WriteToStdout(msg) ? 1 : 0;
}
int x_platform_syscall_create_and_open_temp_file(char* pattern,
const size_t pattern_len) {
std::vector<char> pattern_vec(pattern, pattern + pattern_len);
const auto fd = XPlatform::Syscall::CreateAndOpenTempFile(pattern_vec);
if (fd != -1) {
std::copy_n(pattern_vec.begin(), pattern_len, pattern);
}
return fd;
}
int x_platform_syscall_close_file(const int fd) {
return XPlatform::Syscall::CloseFile(fd);
}
}

View File

@ -24,8 +24,5 @@
*/
int x_platform_syscall_write_to_stdout(const char* msg);
int x_platform_syscall_create_and_open_temp_file(char* pattern,
size_t pattern_len);
int x_platform_syscall_close_file(int fd);
#endif // NATIVE_LIBHDFSPP_LIB_CROSS_PLATFORM_C_API_SYSCALL_H

View File

@ -20,7 +20,6 @@
#define NATIVE_LIBHDFSPP_LIB_CROSS_PLATFORM_SYSCALL
#include <string>
#include <vector>
/**
* The {@link XPlatform} namespace contains components that
@ -85,34 +84,6 @@ class Syscall {
static bool StringCompareIgnoreCase(const std::string& a,
const std::string& b);
/**
* Creates and opens a temporary file with a given {@link pattern}.
* The {@link pattern} must end with a minimum of 6 'X' characters.
* This function will first modify the last 6 'X' characters with
* random character values, which serve as the temporary file name.
* Subsequently opens the file and returns the file descriptor for
* the same. The behaviour of this function is the same as that of
* POSIX mkstemp function. The file must be later closed by the
* application and is not handled by this function.
*
* @param pattern the pattern to be used for the temporary filename.
* @returns an integer representing the file descriptor for the
* opened temporary file. Returns -1 in the case of error and sets
* the global errno with the appropriate error code.
*/
static int CreateAndOpenTempFile(std::vector<char>& pattern);
/**
* Closes the file corresponding to given {@link file_descriptor}.
*
* @param file_descriptor the file descriptor of the file to close.
* @returns a boolean indicating the status of the call to this
* function. true if it's a success, false in the case of an error.
* The global errno is set if the call to this function was not
* successful.
*/
static bool CloseFile(int file_descriptor);
private:
static bool WriteToStdoutImpl(const char* message);
};

View File

@ -21,7 +21,6 @@
#include <unistd.h>
#include <cstring>
#include <vector>
#include "syscall.h"
@ -60,13 +59,3 @@ bool XPlatform::Syscall::StringCompareIgnoreCase(const std::string& a,
const std::string& b) {
return strcasecmp(a.c_str(), b.c_str()) == 0;
}
int XPlatform::Syscall::CreateAndOpenTempFile(std::vector<char>& pattern) {
// Make space for mkstemp to add NULL character at the end
pattern.resize(pattern.size() + 1);
return mkstemp(pattern.data());
}
bool XPlatform::Syscall::CloseFile(const int file_descriptor) {
return close(file_descriptor) == 0;
}

View File

@ -19,14 +19,7 @@
#include <Shlwapi.h>
#include <WinBase.h>
#include <Windows.h>
#include <fcntl.h>
#include <io.h>
#include <share.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <cerrno>
#include <cstdlib>
#include <cstring>
#include "syscall.h"
@ -71,26 +64,3 @@ bool XPlatform::Syscall::StringCompareIgnoreCase(const std::string& a,
const std::string& b) {
return _stricmp(a.c_str(), b.c_str()) == 0;
}
int XPlatform::Syscall::CreateAndOpenTempFile(std::vector<char>& pattern) {
if (_set_errno(0) != 0) {
return -1;
}
// Make space for _mktemp_s to add NULL character at the end
pattern.resize(pattern.size() + 1);
if (_mktemp_s(pattern.data(), pattern.size()) != 0) {
return -1;
}
auto fd{-1};
if (_sopen_s(&fd, pattern.data(), _O_RDWR | _O_CREAT | _O_EXCL, _SH_DENYNO,
_S_IREAD | _S_IWRITE) != 0) {
return -1;
}
return fd;
}
bool XPlatform::Syscall::CloseFile(const int file_descriptor) {
return _close(file_descriptor) == 0;
}

View File

@ -96,13 +96,11 @@ add_executable(node_exclusion_test node_exclusion_test.cc)
target_link_libraries(node_exclusion_test fs gmock_main common ${PROTOBUF_LIBRARIES} ${OPENSSL_LIBRARIES} ${SASL_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT})
add_memcheck_test(node_exclusion node_exclusion_test)
add_executable(configuration_test $<TARGET_OBJECTS:x_platform_obj> configuration_test.cc)
target_include_directories(configuration_test PRIVATE ../lib)
add_executable(configuration_test configuration_test.cc)
target_link_libraries(configuration_test common gmock_main ${CMAKE_THREAD_LIBS_INIT})
add_memcheck_test(configuration configuration_test)
add_executable(hdfs_configuration_test $<TARGET_OBJECTS:x_platform_obj> hdfs_configuration_test.cc)
target_include_directories(hdfs_configuration_test PRIVATE ../lib)
add_executable(hdfs_configuration_test hdfs_configuration_test.cc)
target_link_libraries(hdfs_configuration_test common gmock_main ${CMAKE_THREAD_LIBS_INIT})
add_memcheck_test(hdfs_configuration hdfs_configuration_test)
@ -110,13 +108,11 @@ add_executable(hdfspp_errors_test hdfspp_errors.cc)
target_link_libraries(hdfspp_errors_test common gmock_main bindings_c fs rpc proto common reader connection ${PROTOBUF_LIBRARIES} ${OPENSSL_LIBRARIES} ${SASL_LIBRARIES} gmock_main ${CMAKE_THREAD_LIBS_INIT})
add_memcheck_test(hdfspp_errors hdfspp_errors_test)
add_executable(hdfs_builder_test $<TARGET_OBJECTS:x_platform_obj> hdfs_builder_test.cc)
target_include_directories(hdfs_builder_test PRIVATE ../lib)
add_executable(hdfs_builder_test hdfs_builder_test.cc)
target_link_libraries(hdfs_builder_test test_common gmock_main bindings_c fs rpc proto common reader connection ${PROTOBUF_LIBRARIES} ${OPENSSL_LIBRARIES} ${SASL_LIBRARIES} gmock_main ${CMAKE_THREAD_LIBS_INIT})
add_memcheck_test(hdfs_builder_test hdfs_builder_test)
add_executable(logging_test logging_test.cc $<TARGET_OBJECTS:x_platform_obj>)
target_include_directories(logging_test PRIVATE ../lib)
target_link_libraries(logging_test common gmock_main bindings_c fs rpc proto common reader connection ${PROTOBUF_LIBRARIES} ${OPENSSL_LIBRARIES} ${SASL_LIBRARIES} gmock_main ${CMAKE_THREAD_LIBS_INIT})
add_memcheck_test(logging_test logging_test)
@ -128,8 +124,7 @@ add_executable(user_lock_test user_lock_test.cc)
target_link_libraries(user_lock_test fs gmock_main common ${PROTOBUF_LIBRARIES} ${OPENSSL_LIBRARIES} ${SASL_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT})
add_memcheck_test(user_lock user_lock_test)
add_executable(hdfs_config_connect_bugs_test $<TARGET_OBJECTS:x_platform_obj> hdfs_config_connect_bugs.cc)
target_include_directories(hdfs_config_connect_bugs_test PRIVATE ../lib)
add_executable(hdfs_config_connect_bugs_test hdfs_config_connect_bugs.cc)
target_link_libraries(hdfs_config_connect_bugs_test common gmock_main bindings_c fs rpc proto common reader connection ${PROTOBUF_LIBRARIES} ${OPENSSL_LIBRARIES} ${SASL_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT})
add_memcheck_test(hdfs_config_connect_bugs hdfs_config_connect_bugs_test)

View File

@ -299,12 +299,11 @@ TEST(ConfigurationTest, TestFileReads)
// Single stream
{
TempFile tempFile;
writeSimpleConfig(tempFile.GetFileName(), "key1", "value1");
writeSimpleConfig(tempFile.filename, "key1", "value1");
ConfigurationLoader config_loader;
config_loader.ClearSearchPath();
optional<Configuration> config =
config_loader.LoadFromFile<Configuration>(tempFile.GetFileName());
optional<Configuration> config = config_loader.LoadFromFile<Configuration>(tempFile.filename);
EXPECT_TRUE(config && "Parse first stream");
EXPECT_EQ("value1", config->GetWithDefault("key1", ""));
}
@ -312,18 +311,16 @@ TEST(ConfigurationTest, TestFileReads)
// Multiple files
{
TempFile tempFile;
writeSimpleConfig(tempFile.GetFileName(), "key1", "value1");
writeSimpleConfig(tempFile.filename, "key1", "value1");
ConfigurationLoader loader;
optional<Configuration> config =
loader.LoadFromFile<Configuration>(tempFile.GetFileName());
optional<Configuration> config = loader.LoadFromFile<Configuration>(tempFile.filename);
ASSERT_TRUE(config && "Parse first stream");
EXPECT_EQ("value1", config->GetWithDefault("key1", ""));
TempFile tempFile2;
writeSimpleConfig(tempFile2.GetFileName(), "key2", "value2");
optional<Configuration> config2 =
loader.OverlayResourceFile(*config, tempFile2.GetFileName());
writeSimpleConfig(tempFile2.filename, "key2", "value2");
optional<Configuration> config2 = loader.OverlayResourceFile(*config, tempFile2.filename);
ASSERT_TRUE(config2 && "Parse second stream");
EXPECT_EQ("value1", config2->GetWithDefault("key1", ""));
EXPECT_EQ("value2", config2->GetWithDefault("key2", ""));
@ -353,13 +350,13 @@ TEST(ConfigurationTest, TestFileReads)
{
TempDir tempDir1;
TempFile tempFile1(tempDir1.path + "/file1.xml");
writeSimpleConfig(tempFile1.GetFileName(), "key1", "value1");
writeSimpleConfig(tempFile1.filename, "key1", "value1");
TempDir tempDir2;
TempFile tempFile2(tempDir2.path + "/file2.xml");
writeSimpleConfig(tempFile2.GetFileName(), "key2", "value2");
writeSimpleConfig(tempFile2.filename, "key2", "value2");
TempDir tempDir3;
TempFile tempFile3(tempDir3.path + "/file3.xml");
writeSimpleConfig(tempFile3.GetFileName(), "key3", "value3");
writeSimpleConfig(tempFile3.filename, "key3", "value3");
ConfigurationLoader loader;
loader.SetSearchPath(tempDir1.path + ":" + tempDir2.path + ":" + tempDir3.path);
@ -380,7 +377,7 @@ TEST(ConfigurationTest, TestDefaultConfigs) {
{
TempDir tempDir;
TempFile coreSite(tempDir.path + "/core-site.xml");
writeSimpleConfig(coreSite.GetFileName(), "key1", "value1");
writeSimpleConfig(coreSite.filename, "key1", "value1");
ConfigurationLoader loader;
loader.SetSearchPath(tempDir.path);

View File

@ -21,19 +21,11 @@
#include "hdfspp/config_parser.h"
#include "common/configuration.h"
#include "common/configuration_loader.h"
#include "x-platform/syscall.h"
#include <cstdio>
#include <fstream>
#include <istream>
#include <string>
#include <utility>
#include <vector>
#include <ftw.h>
#include <unistd.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
namespace hdfs {
@ -115,52 +107,24 @@ void writeDamagedConfig(const std::string& filename, Args... args) {
// TempDir: is deleted on destruction
class TempFile {
public:
TempFile() {
std::vector<char> tmp_buf(filename_.begin(), filename_.end());
fd_ = XPlatform::Syscall::CreateAndOpenTempFile(tmp_buf);
EXPECT_NE(fd_, -1);
filename_.assign(tmp_buf.data());
public:
std::string filename;
char fn_buffer[128];
int tempFileHandle;
TempFile() : tempFileHandle(-1) {
strncpy(fn_buffer, "/tmp/test_XXXXXXXXXX", sizeof(fn_buffer));
tempFileHandle = mkstemp(fn_buffer);
EXPECT_NE(-1, tempFileHandle);
filename = fn_buffer;
}
TempFile(std::string fn) : filename_(std::move(fn)) {}
TempFile(const TempFile& other) = default;
TempFile(TempFile&& other) noexcept
: filename_{std::move(other.filename_)}, fd_{other.fd_} {}
TempFile& operator=(const TempFile& other) {
if (&other != this) {
filename_ = other.filename_;
fd_ = other.fd_;
}
return *this;
TempFile(const std::string & fn) : filename(fn), tempFileHandle(-1) {
strncpy(fn_buffer, fn.c_str(), sizeof(fn_buffer));
fn_buffer[sizeof(fn_buffer)-1] = 0;
}
TempFile& operator=(TempFile&& other) noexcept {
if (&other != this) {
filename_ = std::move(other.filename_);
fd_ = other.fd_;
}
return *this;
}
[[nodiscard]] const std::string& GetFileName() const { return filename_; }
~TempFile() {
if (-1 != fd_) {
EXPECT_NE(XPlatform::Syscall::CloseFile(fd_), -1);
}
unlink(filename_.c_str());
}
private:
std::string filename_{"/tmp/test_XXXXXXXXXX"};
int fd_{-1};
~TempFile() { if(-1 != tempFileHandle) close(tempFileHandle); unlink(fn_buffer); }
};
// Callback to remove a directory in the nftw visitor
int nftw_remove(const char *fpath, const struct stat *sb, int typeflag, struct FTW *ftwbuf)
{

View File

@ -45,7 +45,7 @@ TEST(HdfsBuilderTest, TestRead)
{
TempDir tempDir1;
TempFile tempFile1(tempDir1.path + "/core-site.xml");
writeSimpleConfig(tempFile1.GetFileName(), "key1", "value1");
writeSimpleConfig(tempFile1.filename, "key1", "value1");
hdfsBuilder * builder = hdfsNewBuilderFromDirectory(tempDir1.path.c_str());
@ -68,7 +68,7 @@ TEST(HdfsBuilderTest, TestRead)
{
TempDir tempDir1;
TempFile tempFile1(tempDir1.path + "/core-site.xml");
writeSimpleConfig(tempFile1.GetFileName(), "key1", "100");
writeSimpleConfig(tempFile1.filename, "key1", "100");
hdfsBuilder * builder = hdfsNewBuilderFromDirectory(tempDir1.path.c_str());

View File

@ -72,9 +72,9 @@ TEST(HdfsConfigurationTest, TestDefaultConfigs) {
{
TempDir tempDir;
TempFile coreSite(tempDir.path + "/core-site.xml");
writeSimpleConfig(coreSite.GetFileName(), "key1", "value1");
writeSimpleConfig(coreSite.filename, "key1", "value1");
TempFile hdfsSite(tempDir.path + "/hdfs-site.xml");
writeSimpleConfig(hdfsSite.GetFileName(), "key2", "value2");
writeSimpleConfig(hdfsSite.filename, "key2", "value2");
ConfigurationLoader loader;
loader.SetSearchPath(tempDir.path);
@ -89,7 +89,7 @@ TEST(HdfsConfigurationTest, TestDefaultConfigs) {
{
TempDir tempDir;
TempFile coreSite(tempDir.path + "/core-site.xml");
writeSimpleConfig(coreSite.GetFileName(), "key1", "value1");
writeSimpleConfig(coreSite.filename, "key1", "value1");
ConfigurationLoader loader;
loader.SetSearchPath(tempDir.path);
@ -103,7 +103,7 @@ TEST(HdfsConfigurationTest, TestDefaultConfigs) {
{
TempDir tempDir;
TempFile hdfsSite(tempDir.path + "/hdfs-site.xml");
writeSimpleConfig(hdfsSite.GetFileName(), "key2", "value2");
writeSimpleConfig(hdfsSite.filename, "key2", "value2");
ConfigurationLoader loader;
loader.SetSearchPath(tempDir.path);
@ -121,9 +121,9 @@ TEST(HdfsConfigurationTest, TestConfigParserAPI) {
{
TempDir tempDir;
TempFile coreSite(tempDir.path + "/core-site.xml");
writeSimpleConfig(coreSite.GetFileName(), "key1", "value1");
writeSimpleConfig(coreSite.filename, "key1", "value1");
TempFile hdfsSite(tempDir.path + "/hdfs-site.xml");
writeSimpleConfig(hdfsSite.GetFileName(), "key2", "value2");
writeSimpleConfig(hdfsSite.filename, "key2", "value2");
ConfigParser parser(tempDir.path);
@ -142,9 +142,9 @@ TEST(HdfsConfigurationTest, TestConfigParserAPI) {
{
TempDir tempDir;
TempFile coreSite(tempDir.path + "/core-site.xml");
writeSimpleConfig(coreSite.GetFileName(), "key1", "value1");
writeSimpleConfig(coreSite.filename, "key1", "value1");
TempFile hdfsSite(tempDir.path + "/hdfs-site.xml");
writeDamagedConfig(hdfsSite.GetFileName(), "key2", "value2");
writeDamagedConfig(hdfsSite.filename, "key2", "value2");
ConfigParser parser(tempDir.path);

View File

@ -85,21 +85,3 @@ TEST(XPlatformSyscall, StringCompareIgnoreCaseNegative) {
EXPECT_FALSE(XPlatform::Syscall::StringCompareIgnoreCase("abcd", "abcde"));
EXPECT_FALSE(XPlatform::Syscall::StringCompareIgnoreCase("12345", "abcde"));
}
TEST(XPlatformSyscall, CreateAndOpenTempFileBasic) {
std::string pattern("tmp-XXXXXX");
std::vector<char> pattern_vec(pattern.begin(), pattern.end());
const auto fd = XPlatform::Syscall::CreateAndOpenTempFile(pattern_vec);
EXPECT_GT(fd, -1);
EXPECT_TRUE(XPlatform::Syscall::CloseFile(fd));
}
TEST(XPlatformSyscall, CreateAndOpenTempFileNegative) {
std::string pattern("does-not-adhere-to-pattern");
std::vector<char> pattern_vec(pattern.begin(), pattern.end());
const auto fd = XPlatform::Syscall::CreateAndOpenTempFile(pattern_vec);
EXPECT_EQ(fd, -1);
EXPECT_FALSE(XPlatform::Syscall::CloseFile(fd));
}