From c18e2b649691e393f0c6e0fcd9af288b68d7d9b5 Mon Sep 17 00:00:00 2001 From: Patrick Venture Date: Wed, 21 Nov 2018 14:19:28 -0800 Subject: add dynamic library interface to enable testing Add interface defining the methods for dynamic linking to enable testing. Change-Id: If4d090d3cedc019b426435a1f651191803bfc1a9 Signed-off-by: Patrick Venture --- Makefile.am | 4 ++- fs.cpp | 46 ++++++++++++++++++++++++ fs.hpp | 23 ++++++++++++ internal/sys.cpp | 45 +++++++++++++++++++++++ internal/sys.hpp | 54 ++++++++++++++++++++++++++++ main.cpp | 2 +- test/Makefile.am | 7 +++- test/dlsys_mock.hpp | 23 ++++++++++++ test/utils_unittest.cpp | 96 +++++++++++++++++++++++++++++++++++++++++++++++++ utils.cpp | 49 ++++++++++++------------- utils.hpp | 21 ++++++++++- 11 files changed, 339 insertions(+), 31 deletions(-) create mode 100644 fs.cpp create mode 100644 fs.hpp create mode 100644 internal/sys.cpp create mode 100644 internal/sys.hpp create mode 100644 test/dlsys_mock.hpp create mode 100644 test/utils_unittest.cpp diff --git a/Makefile.am b/Makefile.am index c0cbffb..08f5235 100644 --- a/Makefile.am +++ b/Makefile.am @@ -8,7 +8,9 @@ libblobcmds_la_SOURCES = \ manager.cpp \ process.cpp \ crc.cpp \ - utils.cpp + utils.cpp \ + internal/sys.cpp \ + fs.cpp libblobcmds_la_LDFLAGS = \ $(SYSTEMD_LIBS) \ diff --git a/fs.cpp b/fs.cpp new file mode 100644 index 0000000..0c20d83 --- /dev/null +++ b/fs.cpp @@ -0,0 +1,46 @@ +/* + * Copyright 2018 Google Inc. + * + * Licensed 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 "fs.hpp" + +#include +#include +#include + +namespace blobs +{ +namespace fs = std::experimental::filesystem; + +std::vector getLibraryList(const std::string& path, + PathMatcher check) +{ + std::vector output; + + for (const auto& p : fs::recursive_directory_iterator(path)) + { + auto ps = p.path().string(); + + if (check(ps)) + { + output.push_back(ps); + } + } + + /* Algorithm ends up being two-pass, but expectation is a list under 10. */ + return output; +} + +} // namespace blobs diff --git a/fs.hpp b/fs.hpp new file mode 100644 index 0000000..2fb574e --- /dev/null +++ b/fs.hpp @@ -0,0 +1,23 @@ +#pragma once + +#include +#include +#include + +namespace blobs +{ +using PathMatcher = std::function; + +/** + * Returns a list of library paths. Checks against match method. + * + * TODO: Can be dropped if we implement a clean fs wrapper for test injection. + * + * @param[in] path - the path to search + * @param[in] check - the function to call to check the path + * @return a list of paths that match the criteria + */ +std::vector getLibraryList(const std::string& path, + PathMatcher check); + +} // namespace blobs diff --git a/internal/sys.cpp b/internal/sys.cpp new file mode 100644 index 0000000..2aec78b --- /dev/null +++ b/internal/sys.cpp @@ -0,0 +1,45 @@ +/* + * Copyright 2018 Google Inc. + * + * Licensed 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 "sys.hpp" + +#include + +namespace blobs +{ + +namespace internal +{ + +const char* DlSysImpl::dlerror() const +{ + return ::dlerror(); +} + +void* DlSysImpl::dlopen(const char* filename, int flags) const +{ + return ::dlopen(filename, flags); +} + +void* DlSysImpl::dlsym(void* handle, const char* symbol) const +{ + return ::dlsym(handle, symbol); +} + +DlSysImpl dlsys_impl; + +} // namespace internal +} // namespace blobs diff --git a/internal/sys.hpp b/internal/sys.hpp new file mode 100644 index 0000000..e0a02bf --- /dev/null +++ b/internal/sys.hpp @@ -0,0 +1,54 @@ +#pragma once + +namespace blobs +{ + +namespace internal +{ +/** + * Interface to the dynamic library loader. + */ +class DlSysInterface +{ + public: + virtual ~DlSysInterface() = default; + + /** + * obtain error diagnostic for functions in the dlopen API + * + * @return the error details + */ + virtual const char* dlerror() const = 0; + + /** + * open a shared object + * + * @param[in] filename - the path to the shared object or the filename to + * for searching + * @param[in] flags - the flags + * @return a handle to the shared object (null on failure) + */ + virtual void* dlopen(const char* filename, int flags) const = 0; + + /** + * obtain address of a symbol in a shared object or executable + * + * @param[in] handle - pointer to shared object + * @param[in] symbol - name of the symbol to find + * @return the address of the symbol if found (null otherwise) + */ + virtual void* dlsym(void* handle, const char* symbol) const = 0; +}; + +class DlSysImpl : public DlSysInterface +{ + public: + const char* dlerror() const override; + void* dlopen(const char* filename, int flags) const override; + void* dlsym(void* handle, const char* symbol) const override; +}; + +extern DlSysImpl dlsys_impl; + +} // namespace internal +} // namespace blobs diff --git a/main.cpp b/main.cpp index 5fdb53d..ee236d9 100644 --- a/main.cpp +++ b/main.cpp @@ -78,7 +78,7 @@ void setupBlobGlobalHandler() /* Install handlers. */ try { - loadLibraries(expectedHandlerPath); + loadLibraries(getBlobManager(), expectedHandlerPath); } catch (const std::exception& e) { diff --git a/test/Makefile.am b/test/Makefile.am index 5e0cdb0..0d36509 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -35,7 +35,9 @@ check_PROGRAMS = \ manager_read_unittest \ manager_writemeta_unittest \ process_unittest \ - crc_unittest + crc_unittest \ + utils_unittest + TESTS = $(check_PROGRAMS) ipmi_unittest_SOURCES = ipmi_unittest.cpp @@ -116,3 +118,6 @@ process_unittest_LDADD = $(top_builddir)/process.o $(top_builddir)/ipmi.o \ crc_unittest_SOURCES = crc_unittest.cpp crc_unittest_LDADD = $(top_builddir)/crc.o + +utils_unittest_SOURCES = utils_unittest.cpp +utils_unittest_LDADD = $(top_builddir)/utils.o $(PHOSPHOR_LOGGING_LIBS) diff --git a/test/dlsys_mock.hpp b/test/dlsys_mock.hpp new file mode 100644 index 0000000..795b40e --- /dev/null +++ b/test/dlsys_mock.hpp @@ -0,0 +1,23 @@ +#pragma once + +#include "internal/sys.hpp" + +#include + +namespace blobs +{ +namespace internal +{ + +class InternalDlSysMock : public DlSysInterface +{ + public: + virtual ~InternalDlSysMock() = default; + + MOCK_CONST_METHOD0(dlerror, const char*()); + MOCK_CONST_METHOD2(dlopen, void*(const char*, int)); + MOCK_CONST_METHOD2(dlsym, void*(void*, const char*)); +}; + +} // namespace internal +} // namespace blobs diff --git a/test/utils_unittest.cpp b/test/utils_unittest.cpp new file mode 100644 index 0000000..8a0cdca --- /dev/null +++ b/test/utils_unittest.cpp @@ -0,0 +1,96 @@ +#include "dlsys_mock.hpp" +#include "fs.hpp" +#include "utils.hpp" + +#include +#include +#include +#include +#include +#include + +#include + +namespace fs = std::experimental::filesystem; + +namespace blobs +{ +using ::testing::_; +using ::testing::Return; +using ::testing::StrEq; +using ::testing::StrictMock; + +std::vector* returnList = nullptr; + +std::vector getLibraryList(const std::string& path, + PathMatcher check) +{ + return (returnList) ? *returnList : std::vector(); +} + +std::unique_ptr factoryReturn; + +std::unique_ptr fakeFactory() +{ + return std::move(factoryReturn); +} + +TEST(UtilLoadLibraryTest, NoFilesFound) +{ + /* Verify nothing special happens when there are no files found. */ + + StrictMock dlsys; + StrictMock manager; + + loadLibraries(&manager, "", &dlsys); +} + +TEST(UtilLoadLibraryTest, OneFileFoundIsLibrary) +{ + /* Verify if it finds a library, and everything works, it'll regsiter it. + */ + + std::vector files = {"this.fake"}; + returnList = &files; + + StrictMock dlsys; + StrictMock manager; + void* handle = reinterpret_cast(0x01); + auto blobMock = std::make_unique(); + + factoryReturn = std::move(blobMock); + + EXPECT_CALL(dlsys, dlopen(_, _)).WillOnce(Return(handle)); + + EXPECT_CALL(dlsys, dlerror()).Times(2).WillRepeatedly(Return(nullptr)); + + EXPECT_CALL(dlsys, dlsym(handle, StrEq("createHandler"))) + .WillOnce(Return(reinterpret_cast(fakeFactory))); + + EXPECT_CALL(manager, registerHandler(_)); + + loadLibraries(&manager, "", &dlsys); +} + +TEST(UtilLibraryMatchTest, TestAll) +{ + struct LibraryMatch + { + std::string name; + bool expectation; + }; + + std::vector tests = { + {"libblobcmds.0.0.1", false}, {"libblobcmds.0.0", false}, + {"libblobcmds.0", false}, {"libblobcmds.10", false}, + {"libblobcmds.a", false}, {"libcmds.so.so.0", true}, + {"libcmds.so.0", true}, {"libcmds.so.0.0", false}, + {"libcmds.so.0.0.10", false}, {"libblobs.so.1000", true}}; + + for (const auto& test : tests) + { + EXPECT_EQ(test.expectation, matchBlobHandler(test.name)); + } +} + +} // namespace blobs diff --git a/utils.cpp b/utils.cpp index 1e1ef91..24166ca 100644 --- a/utils.cpp +++ b/utils.cpp @@ -16,60 +16,55 @@ #include "utils.hpp" +#include "fs.hpp" + #include #include -#include #include #include #include +#include +#include namespace blobs { -namespace fs = std::experimental::filesystem; using namespace phosphor::logging; -using HandlerFactory = std::unique_ptr (*)(); +bool matchBlobHandler(const std::string& filename) +{ + return std::regex_match(filename, std::regex(".+\\.so\\.\\d+$")); +} -void loadLibraries(const std::string& path) +void loadLibraries(ManagerInterface* manager, const std::string& path, + const internal::DlSysInterface* sys) { void* libHandle = NULL; HandlerFactory factory; - auto* manager = getBlobManager(); - for (const auto& p : fs::recursive_directory_iterator(path)) - { - auto ps = p.path().string(); - - /* The bitbake recipe symlinks the library lib*.so.? into the folder - * only, and not the other names, .so, .so.?.?, .so.?.?.? - * - * Therefore only care if it's lib*.so.? - */ - if (!std::regex_match(ps, std::regex(".+\\.so\\.\\d+$"))) - { - continue; - } + std::vector libs = getLibraryList(path, matchBlobHandler); - libHandle = dlopen(ps.c_str(), RTLD_NOW | RTLD_GLOBAL); + for (const auto& p : libs) + { + libHandle = sys->dlopen(p.c_str(), RTLD_NOW | RTLD_GLOBAL); if (!libHandle) { - log("ERROR opening", entry("HANDLER=%s", ps.c_str()), - entry("ERROR=%s", dlerror())); + log("ERROR opening", entry("HANDLER=%s", p.c_str()), + entry("ERROR=%s", sys->dlerror())); continue; } - dlerror(); /* Clear any previous error. */ + sys->dlerror(); /* Clear any previous error. */ - factory = - reinterpret_cast(dlsym(libHandle, "createHandler")); + factory = reinterpret_cast( + sys->dlsym(libHandle, "createHandler")); - const char* error = dlerror(); + const char* error = sys->dlerror(); if (error) { log("ERROR loading symbol", - entry("HANDLER=%s", ps.c_str()), + entry("HANDLER=%s", p.c_str()), entry("ERROR=%s", error)); continue; } @@ -78,7 +73,7 @@ void loadLibraries(const std::string& path) if (!result) { log("Unable to create handler", - entry("HANDLER=%s", ps.c_str())); + entry("HANDLER=%s", p.c_str())); continue; } diff --git a/utils.hpp b/utils.hpp index 9b75a40..50d320e 100644 --- a/utils.hpp +++ b/utils.hpp @@ -1,15 +1,34 @@ #pragma once +#include "internal/sys.hpp" + +#include +#include #include namespace blobs { +using HandlerFactory = std::unique_ptr (*)(); + +/** + * The bitbake recipe symlinks the library lib*.so.? into the folder + * only, and not the other names, .so, .so.?.?, .so.?.?.? + * + * Therefore only care if it's lib*.so.? + * + * @param[in] the path to check. + * @return true if matches, false otherwise + */ +bool matchBlobHandler(const std::string& filename); /** * @brief Given a path, find libraries (*.so only) and load them. * + * @param[in] manager - pointer to a manager * @param[in] paths - list of fully qualified paths to libraries to load. + * @param[in] sys - pointer to implementation of the dlsys interface. */ -void loadLibraries(const std::string& path); +void loadLibraries(ManagerInterface* manager, const std::string& path, + const internal::DlSysInterface* sys = &internal::dlsys_impl); } // namespace blobs -- cgit v1.2.3