From 11671c2bf6323cbac5885f718c966eec8264e7bc Mon Sep 17 00:00:00 2001 From: James Date: Sat, 9 Jan 2016 23:52:45 -0500 Subject: [PATCH] HDFS-9627. libhdfs++: Add mechanism to retrieve human readable error messages through the C API. Contributed by James Clampffer. --- .../libhdfspp/include/libhdfspp/hdfs_ext.h | 67 ++++++++++ .../include/libhdfspp/{hdfs.h => hdfspp.h} | 4 +- .../native/libhdfspp/lib/bindings/c/hdfs.cc | 25 ++++ .../libhdfspp/lib/common/hdfs_public_api.h | 2 +- .../libhdfspp/lib/fs/bad_datanode_tracker.h | 2 +- .../main/native/libhdfspp/lib/fs/filesystem.h | 2 +- .../native/libhdfspp/tests/CMakeLists.txt | 6 +- .../native/libhdfspp/tests/hdfspp_errors.cc | 117 ++++++++++++++++++ 8 files changed, 219 insertions(+), 6 deletions(-) create mode 100644 hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/libhdfspp/hdfs_ext.h rename hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/libhdfspp/{hdfs.h => hdfspp.h} (98%) create mode 100644 hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/hdfspp_errors.cc diff --git a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/libhdfspp/hdfs_ext.h b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/libhdfspp/hdfs_ext.h new file mode 100644 index 00000000000..2d793f4dbea --- /dev/null +++ b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/libhdfspp/hdfs_ext.h @@ -0,0 +1,67 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#ifndef LIBHDFSPP_HDFS_HDFSEXT +#define LIBHDFSPP_HDFS_HDFSEXT + +/* get typdefs and #defines from libhdfs' hdfs.h to stay consistent */ +#include + +/** + * Note: The #defines below are copied directly from libhdfs' + * hdfs.h. LIBHDFS_EXTERNAL gets explicitly #undefed at the + * end of the file so it must be redefined here. + **/ + +#ifdef WIN32 + #ifdef LIBHDFS_DLL_EXPORT + #define LIBHDFS_EXTERNAL __declspec(dllexport) + #elif LIBHDFS_DLL_IMPORT + #define LIBHDFS_EXTERNAL __declspec(dllimport) + #else + #define LIBHDFS_EXTERNAL + #endif +#else + #ifdef LIBHDFS_DLL_EXPORT + #define LIBHDFS_EXTERNAL __attribute__((visibility("default"))) + #elif LIBHDFS_DLL_IMPORT + #define LIBHDFS_EXTERNAL __attribute__((visibility("default"))) + #else + #define LIBHDFS_EXTERNAL + #endif +#endif + + +/** + * Keep C bindings that are libhdfs++ specific in here. + **/ + +extern "C" { +/** + * Reads the last error, if any, that happened in this thread + * into the user supplied buffer. + * @param buf A chunk of memory with room for the error string. + * @param len Size of the buffer, if the message is longer than + * len len-1 bytes of the message will be copied. + **/ + +LIBHDFS_EXTERNAL +void hdfsGetLastError(char *buf, int len); + + +} /* end extern "C" */ +#endif diff --git a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/libhdfspp/hdfs.h b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/libhdfspp/hdfspp.h similarity index 98% rename from hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/libhdfspp/hdfs.h rename to hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/libhdfspp/hdfspp.h index 7dc3f88deab..ebcf2272d12 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/libhdfspp/hdfs.h +++ b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/libhdfspp/hdfspp.h @@ -15,8 +15,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#ifndef LIBHDFSPP_HDFS_H_ -#define LIBHDFSPP_HDFS_H_ +#ifndef LIBHDFSPP_HDFSPP_H_ +#define LIBHDFSPP_HDFSPP_H_ #include "libhdfspp/options.h" #include "libhdfspp/status.h" diff --git a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/bindings/c/hdfs.cc b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/bindings/c/hdfs.cc index d23c7b0d637..0fc02b4bfa1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/bindings/c/hdfs.cc +++ b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/bindings/c/hdfs.cc @@ -19,9 +19,12 @@ #include "fs/filesystem.h" #include +#include + #include #include #include +#include using namespace hdfs; @@ -49,9 +52,31 @@ struct hdfsFile_internal { std::unique_ptr file_; }; +/* Keep thread local copy of last error string */ +thread_local std::string errstr; + +/* Fetch last error that happened in this thread */ +void hdfsGetLastError(char *buf, int len) { + if(nullptr == buf || len < 1 || errstr.empty()) { + return; + } + + /* leave space for a trailing null */ + size_t copylen = std::min((size_t)errstr.size(), (size_t)len); + if(copylen == (size_t)len) { + copylen--; + } + + strncpy(buf, errstr.c_str(), copylen); + + /* stick in null */ + buf[copylen] = 0; +} + /* Error handling with optional debug to stderr */ static void ReportError(int errnum, std::string msg) { errno = errnum; + errstr = msg; #ifdef LIBHDFSPP_C_API_ENABLE_DEBUG std::cerr << "Error: errno=" << strerror(errnum) << " message=\"" << msg << "\"" << std::endl; diff --git a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/hdfs_public_api.h b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/hdfs_public_api.h index 95567c0d003..acd96024e80 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/hdfs_public_api.h +++ b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/hdfs_public_api.h @@ -19,7 +19,7 @@ #ifndef COMMON_HDFS_PUBLIC_API_H_ #define COMMON_HDFS_PUBLIC_API_H_ -#include "libhdfspp/hdfs.h" +#include "libhdfspp/hdfspp.h" #include diff --git a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/bad_datanode_tracker.h b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/bad_datanode_tracker.h index f56519221f0..72855dc58d9 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/bad_datanode_tracker.h +++ b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/bad_datanode_tracker.h @@ -26,7 +26,7 @@ #include #include "libhdfspp/options.h" -#include "libhdfspp/hdfs.h" +#include "libhdfspp/hdfspp.h" namespace hdfs { diff --git a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/filesystem.h b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/filesystem.h index d78df817974..df618721abb 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/filesystem.h +++ b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/filesystem.h @@ -21,7 +21,7 @@ #include "filehandle.h" #include "common/hdfs_public_api.h" #include "common/async_stream.h" -#include "libhdfspp/hdfs.h" +#include "libhdfspp/hdfspp.h" #include "fs/bad_datanode_tracker.h" #include "rpc/rpc_engine.h" #include "reader/block_reader.h" diff --git a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/CMakeLists.txt b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/CMakeLists.txt index 3dee80110fe..b1d09894ccd 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/CMakeLists.txt +++ b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/CMakeLists.txt @@ -86,7 +86,11 @@ add_memcheck_test(configuration configuration_test) add_executable(hdfs_configuration_test hdfs_configuration_test.cc) target_link_libraries(hdfs_configuration_test common gmock_main ${CMAKE_THREAD_LIBS_INIT}) -add_test(hdfs_configuration hdfs_configuration_test) +add_memcheck_test(hdfs_configuration hdfs_configuration_test) + +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} gmock_main ${CMAKE_THREAD_LIBS_INIT}) +add_memcheck_test(hdfspp_errors hdfspp_errors_test) #This test requires a great deal of Hadoop Java infrastructure to run. if(HADOOP_BUILD) diff --git a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/hdfspp_errors.cc b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/hdfspp_errors.cc new file mode 100644 index 00000000000..0cb65455892 --- /dev/null +++ b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/hdfspp_errors.cc @@ -0,0 +1,117 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + +#include +#include + +#include +#include + +#include +#include + +using ::testing::_; +using ::testing::InvokeArgument; +using ::testing::Return; + +/* Don't need a real minidfs cluster since this just passes invalid params. */ + +TEST(HdfsppErrors, NullFileSystem) { + + char buf[4096]; + + hdfsFS fs = nullptr; + hdfsFile fd = reinterpret_cast(1); + + tSize res = hdfsRead(fs, fd, buf, 4096); + ASSERT_EQ(res, -1); + + hdfsGetLastError(buf, 4096); + + ASSERT_EQ(std::string(buf), "Cannot perform FS operations with null FS handle."); +} + +TEST(HdfsppErrors, NullFileHandle) { + char buf[4096]; + + hdfsFS fs = reinterpret_cast(1); + hdfsFile fd = nullptr; + + tSize res = hdfsRead(fs, fd, buf, 4096); + ASSERT_EQ(res, -1); + + hdfsGetLastError(buf, 4096); + + ASSERT_EQ(std::string(buf), "Cannot perform FS operations with null File handle."); +} + +TEST(HdfsppErrors, ZeroLength) { + char buf[1]; + buf[0] = 0; + + hdfsFS fs = reinterpret_cast(1); + hdfsFile fd = nullptr; + + tSize res = hdfsRead(fs, fd, buf, 1); + ASSERT_EQ(res, -1); + + hdfsGetLastError(buf, 0); + + ASSERT_EQ(std::string(buf), ""); +} + +TEST(HdfsppErrors, NegativeLength) { + char buf[1]; + buf[0] = 0; + + hdfsFS fs = reinterpret_cast(1); + hdfsFile fd = nullptr; + + tSize res = hdfsRead(fs, fd, buf, 1); + ASSERT_EQ(res, -1); + + hdfsGetLastError(buf, -1); + + ASSERT_EQ(std::string(buf), ""); +} + +TEST(HdfsppErrors, MessageTruncation) { + char buf[4096]; + + hdfsFS fs = reinterpret_cast(1); + hdfsFile fd = nullptr; + + tSize res = hdfsRead(fs, fd, buf, 4096); + ASSERT_EQ(res, -1); + + hdfsGetLastError(buf, 10); + + ASSERT_EQ(std::string(buf), "Cannot pe"); +} + +int main(int argc, char *argv[]) { + // The following line must be executed to initialize Google Mock + // (and Google Test) before running the tests. + ::testing::InitGoogleMock(&argc, argv); + int exit_code = RUN_ALL_TESTS(); + google::protobuf::ShutdownProtobufLibrary(); + + return exit_code; +} +