Skip to content

Commit 2b1da15

Browse files
HDFS-15971. Make mkstemp cross platform
* mkstemp isn't available in Visual C++. This PR implements the necessary cross platform implementation for mkstemp.
1 parent 2bd810a commit 2b1da15

9 files changed

Lines changed: 132 additions & 21 deletions

File tree

hadoop-hdfs-project/hadoop-hdfs-native-client/src/CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ function(build_libhdfs_test NAME LIBRARY)
8989
list(APPEND FILES ${CMAKE_SOURCE_DIR}/main/native/libhdfs-tests/${FIL})
9090
endif()
9191
endforeach()
92-
add_executable("${NAME}_${LIBRARY}" ${FILES})
92+
add_executable("${NAME}_${LIBRARY}" $<TARGET_OBJECTS:x_platform_obj_c_api> $<TARGET_OBJECTS:x_platform_obj> ${FILES})
93+
target_include_directories("${NAME}_${LIBRARY}" PRIVATE main/native/libhdfspp/lib)
9394
endfunction()
9495

9596
function(add_libhdfs_test NAME LIBRARY)

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs-tests/test_libhdfs_mini_stress.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "hdfspp/hdfs_ext.h"
2323
#include "native_mini_dfs.h"
2424
#include "os/thread.h"
25+
#include "x-platform/c-api/syscall.h"
2526

2627
#include <errno.h>
2728
#include <inttypes.h>
@@ -126,7 +127,8 @@ static int hdfsCurlData(const char *host, const tPort port, const char *dirNm,
126127
EXPECT_NONNULL(pw = getpwuid(uid));
127128

128129
int fd = -1;
129-
EXPECT_NONNEGATIVE(fd = mkstemp(tmpFile));
130+
EXPECT_NONNEGATIVE(fd = x_platform_syscall_create_and_open_temp_file(
131+
tmpFile, sizeof tmpFile));
130132

131133
tSize sz = 0;
132134
while (sz < fileSz) {

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/x-platform/c-api/syscall.cc

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,26 @@
1818

1919
#include "x-platform/syscall.h"
2020

21-
extern "C" int x_platform_syscall_write_to_stdout(const char* msg) {
21+
#include <algorithm>
22+
#include <vector>
23+
24+
extern "C" {
25+
int x_platform_syscall_write_to_stdout(const char* msg) {
2226
return XPlatform::Syscall::WriteToStdout(msg) ? 1 : 0;
2327
}
28+
29+
int x_platform_syscall_create_and_open_temp_file(char* pattern,
30+
const size_t pattern_len) {
31+
std::vector<char> pattern_vec(pattern, pattern + pattern_len);
32+
33+
const auto fd = XPlatform::Syscall::CreateAndOpenTempFile(pattern_vec);
34+
if (fd != -1) {
35+
std::copy_n(pattern_vec.begin(), pattern_len, pattern);
36+
}
37+
return fd;
38+
}
39+
40+
int x_platform_syscall_close_file(const int fd) {
41+
return XPlatform::Syscall::CloseFile(fd);
42+
}
43+
}

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/x-platform/c-api/syscall.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,8 @@
2424
*/
2525

2626
int x_platform_syscall_write_to_stdout(const char* msg);
27+
int x_platform_syscall_create_and_open_temp_file(char* pattern,
28+
size_t pattern_len);
29+
int x_platform_syscall_close_file(int fd);
2730

2831
#endif // NATIVE_LIBHDFSPP_LIB_CROSS_PLATFORM_C_API_SYSCALL_H

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/x-platform/syscall.h

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#define NATIVE_LIBHDFSPP_LIB_CROSS_PLATFORM_SYSCALL
2121

2222
#include <string>
23+
#include <vector>
2324

2425
/**
2526
* The {@link XPlatform} namespace contains components that
@@ -84,6 +85,34 @@ class Syscall {
8485
static bool StringCompareIgnoreCase(const std::string& a,
8586
const std::string& b);
8687

88+
/**
89+
* Creates and opens a temporary file with a given {@link pattern}.
90+
* The {@link pattern} must end with a minimum of 6 'X' characters.
91+
* This function will first modify the last 6 'X' characters with
92+
* random character values, which serve as the temporary file name.
93+
* Subsequently opens the file and returns the file descriptor for
94+
* the same. The behaviour of this function is the same as that of
95+
* POSIX mkstemp function. The file must be later closed by the
96+
* application and is not handled by this function.
97+
*
98+
* @param pattern the pattern to be used for the temporary filename.
99+
* @returns an integer representing the file descriptor for the
100+
* opened temporary file. Returns -1 in the case of error and sets
101+
* the global errno with the appropriate error code.
102+
*/
103+
static int CreateAndOpenTempFile(std::vector<char>& pattern);
104+
105+
/**
106+
* Closes the file corresponding to given {@link file_descriptor}.
107+
*
108+
* @param file_descriptor the file descriptor of the file to close.
109+
* @returns a boolean indicating the status of the call to this
110+
* function. true if it's a success, false in the case of an error.
111+
* The global errno is set if the call to this function was not
112+
* successful.
113+
*/
114+
static bool CloseFile(int file_descriptor);
115+
87116
private:
88117
static bool WriteToStdoutImpl(const char* message);
89118
};

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/x-platform/syscall_linux.cc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <unistd.h>
2222

2323
#include <cstring>
24+
#include <vector>
2425

2526
#include "syscall.h"
2627

@@ -54,3 +55,13 @@ bool XPlatform::Syscall::StringCompareIgnoreCase(const std::string& a,
5455
const std::string& b) {
5556
return strcasecmp(a.c_str(), b.c_str()) == 0;
5657
}
58+
59+
int XPlatform::Syscall::CreateAndOpenTempFile(std::vector<char>& pattern) {
60+
// Make space for mkstemp to add NULL character at the end
61+
pattern.resize(pattern.size() + 1);
62+
return mkstemp(pattern.data());
63+
}
64+
65+
bool XPlatform::Syscall::CloseFile(const int file_descriptor) {
66+
return close(file_descriptor) == 0;
67+
}

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/x-platform/syscall_windows.cc

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,14 @@
1919
#include <Shlwapi.h>
2020
#include <WinBase.h>
2121
#include <Windows.h>
22+
#include <fcntl.h>
23+
#include <io.h>
24+
#include <share.h>
25+
#include <sys/stat.h>
26+
#include <sys/types.h>
2227

28+
#include <cerrno>
29+
#include <cstdlib>
2330
#include <cstring>
2431

2532
#include "syscall.h"
@@ -64,3 +71,26 @@ bool XPlatform::Syscall::StringCompareIgnoreCase(const std::string& a,
6471
const std::string& b) {
6572
return _stricmp(a.c_str(), b.c_str()) == 0;
6673
}
74+
75+
int XPlatform::Syscall::CreateAndOpenTempFile(std::vector<char>& pattern) {
76+
if (_set_errno(0) != 0) {
77+
return -1;
78+
}
79+
80+
// Make space for _mktemp_s to add NULL character at the end
81+
pattern.resize(pattern.size() + 1);
82+
if (_mktemp_s(pattern.data(), pattern.size()) != 0) {
83+
return -1;
84+
}
85+
86+
auto fd{-1};
87+
if (_sopen_s(&fd, pattern.data(), _O_RDWR | _O_CREAT | _O_EXCL, _SH_DENYNO,
88+
_S_IREAD | _S_IWRITE) != 0) {
89+
return -1;
90+
}
91+
return fd;
92+
}
93+
94+
bool XPlatform::Syscall::CloseFile(const int file_descriptor) {
95+
return _close(file_descriptor) == 0;
96+
}

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/CMakeLists.txt

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,23 +96,27 @@ add_executable(node_exclusion_test node_exclusion_test.cc)
9696
target_link_libraries(node_exclusion_test fs gmock_main common ${PROTOBUF_LIBRARIES} ${OPENSSL_LIBRARIES} ${SASL_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT})
9797
add_memcheck_test(node_exclusion node_exclusion_test)
9898

99-
add_executable(configuration_test configuration_test.cc)
99+
add_executable(configuration_test $<TARGET_OBJECTS:x_platform_obj> configuration_test.cc)
100+
target_include_directories(configuration_test PRIVATE ../lib)
100101
target_link_libraries(configuration_test common gmock_main ${CMAKE_THREAD_LIBS_INIT})
101102
add_memcheck_test(configuration configuration_test)
102103

103-
add_executable(hdfs_configuration_test hdfs_configuration_test.cc)
104+
add_executable(hdfs_configuration_test $<TARGET_OBJECTS:x_platform_obj> hdfs_configuration_test.cc)
105+
target_include_directories(hdfs_configuration_test PRIVATE ../lib)
104106
target_link_libraries(hdfs_configuration_test common gmock_main ${CMAKE_THREAD_LIBS_INIT})
105107
add_memcheck_test(hdfs_configuration hdfs_configuration_test)
106108

107109
add_executable(hdfspp_errors_test hdfspp_errors.cc)
108110
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})
109111
add_memcheck_test(hdfspp_errors hdfspp_errors_test)
110112

111-
add_executable(hdfs_builder_test hdfs_builder_test.cc)
113+
add_executable(hdfs_builder_test $<TARGET_OBJECTS:x_platform_obj> hdfs_builder_test.cc)
114+
target_include_directories(hdfs_builder_test PRIVATE ../lib)
112115
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})
113116
add_memcheck_test(hdfs_builder_test hdfs_builder_test)
114117

115118
add_executable(logging_test logging_test.cc $<TARGET_OBJECTS:x_platform_obj>)
119+
target_include_directories(logging_test PRIVATE ../lib)
116120
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})
117121
add_memcheck_test(logging_test logging_test)
118122

@@ -124,7 +128,8 @@ add_executable(user_lock_test user_lock_test.cc)
124128
target_link_libraries(user_lock_test fs gmock_main common ${PROTOBUF_LIBRARIES} ${OPENSSL_LIBRARIES} ${SASL_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT})
125129
add_memcheck_test(user_lock user_lock_test)
126130

127-
add_executable(hdfs_config_connect_bugs_test hdfs_config_connect_bugs.cc)
131+
add_executable(hdfs_config_connect_bugs_test $<TARGET_OBJECTS:x_platform_obj> hdfs_config_connect_bugs.cc)
132+
target_include_directories(hdfs_config_connect_bugs_test PRIVATE ../lib)
128133
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})
129134
add_memcheck_test(hdfs_config_connect_bugs hdfs_config_connect_bugs_test)
130135

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/configuration_test.h

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,17 @@
2121
#include "hdfspp/config_parser.h"
2222
#include "common/configuration.h"
2323
#include "common/configuration_loader.h"
24+
#include "x-platform/syscall.h"
25+
2426
#include <cstdio>
2527
#include <fstream>
2628
#include <istream>
29+
#include <vector>
30+
2731
#include <ftw.h>
32+
#include <unistd.h>
2833
#include <gmock/gmock.h>
34+
#include <gtest/gtest.h>
2935

3036
namespace hdfs {
3137

@@ -107,23 +113,27 @@ void writeDamagedConfig(const std::string& filename, Args... args) {
107113

108114
// TempDir: is deleted on destruction
109115
class TempFile {
110-
public:
111-
std::string filename;
112-
char fn_buffer[128];
113-
int tempFileHandle;
114-
TempFile() : tempFileHandle(-1) {
115-
strncpy(fn_buffer, "/tmp/test_XXXXXXXXXX", sizeof(fn_buffer));
116-
tempFileHandle = mkstemp(fn_buffer);
117-
EXPECT_NE(-1, tempFileHandle);
118-
filename = fn_buffer;
116+
public:
117+
TempFile() {
118+
std::vector<char> tmp_buf(filename.begin(), filename.end());
119+
fd = XPlatform::Syscall::CreateAndOpenTempFile(tmp_buf);
120+
EXPECT_NE(fd, -1);
121+
filename.assign(tmp_buf.data());
119122
}
120-
TempFile(const std::string & fn) : filename(fn), tempFileHandle(-1) {
121-
strncpy(fn_buffer, fn.c_str(), sizeof(fn_buffer));
122-
fn_buffer[sizeof(fn_buffer)-1] = 0;
123+
124+
TempFile(const std::string& fn) : filename(fn) {}
125+
126+
~TempFile() {
127+
if (-1 != fd) {
128+
EXPECT_NE(XPlatform::Syscall::CloseFile(fd), -1);
129+
}
130+
131+
unlink(filename.c_str());
123132
}
124-
~TempFile() { if(-1 != tempFileHandle) close(tempFileHandle); unlink(fn_buffer); }
125-
};
126133

134+
std::string filename{"/tmp/test_XXXXXXXXXX"};
135+
int fd{-1};
136+
};
127137

128138
// Callback to remove a directory in the nftw visitor
129139
int nftw_remove(const char *fpath, const struct stat *sb, int typeflag, struct FTW *ftwbuf)

0 commit comments

Comments
 (0)