Skip to content

Commit

Permalink
Replace std::string pointer by reference and remove unused function D…
Browse files Browse the repository at this point in the history
…LSym

Signed-off-by: Shreyas Atre <[email protected]>
  • Loading branch information
SAtacker committed Jan 27, 2024
1 parent e61e7ca commit f6d467d
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 26 deletions.
4 changes: 2 additions & 2 deletions lib/Interpreter/DynamicLibraryManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ namespace Cpp {
// TODO: !permanent case

std::string errMsg;
DyLibHandle dyLibHandle = platform::DLOpen(canonicalLoadedLib, &errMsg);
DyLibHandle dyLibHandle = platform::DLOpen(canonicalLoadedLib, errMsg);
if (!dyLibHandle) {
// We emit callback to LibraryLoadingFailed when we get error with error message.
//TODO: Implement callbacks
Expand Down Expand Up @@ -403,7 +403,7 @@ namespace Cpp {
// TODO: !permanent case

std::string errMsg;
platform::DLClose(dyLibHandle, &errMsg);
platform::DLClose(dyLibHandle, errMsg);
if (!errMsg.empty()) {
LLVM_DEBUG(dbgs() << "cling::DynamicLibraryManager::unloadLibrary(): "
<< errMsg << '\n');
Expand Down
27 changes: 6 additions & 21 deletions lib/Interpreter/Paths.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,41 +107,26 @@ namespace platform {
}

#if defined(LLVM_ON_UNIX)
static void DLErr(std::string* Err) {
if (!Err)
static void DLErr(std::string& Err) {
if (Err.empty())
return;
if (const char* DyLibError = ::dlerror())
*Err = DyLibError;
Err = std::string(DyLibError);

Check warning on line 114 in lib/Interpreter/Paths.cpp

View check run for this annotation

Codecov / codecov/patch

lib/Interpreter/Paths.cpp#L114

Added line #L114 was not covered by tests
}

void* DLOpen(const std::string& Path, std::string* Err /* = nullptr */) {
void* DLOpen(const std::string& Path, std::string& Err /* = nullptr */) {
void* Lib = ::dlopen(Path.c_str(), RTLD_LAZY | RTLD_GLOBAL);
DLErr(Err);
return Lib;
}

void* DLSym(const std::string& Name, std::string* Err /* = nullptr*/) {
if (void* Self = ::dlopen(nullptr, RTLD_GLOBAL)) {
// get dlopen error if there is one
DLErr(Err);
void* Sym = ::dlsym(Self, Name.c_str());
// overwrite error if dlsym caused one
DLErr(Err);
// only get dlclose error if dlopen & dlsym haven't emited one
DLClose(Self, Err && Err->empty() ? Err : nullptr);
return Sym;
}
DLErr(Err);
return nullptr;
}

void DLClose(void* Lib, std::string* Err /* = nullptr*/) {
void DLClose(void* Lib, std::string& Err /* = nullptr*/) {
::dlclose(Lib);
DLErr(Err);
}
#elif defined(_WIN32)

void* DLOpen(const std::string& Path, std::string* Err /* = nullptr */) {
void* DLOpen(const std::string& Path, std::string& Err /* = nullptr */) {
auto lib = llvm::sys::DynamicLibrary::getLibrary(Path.c_str(), Err);
return lib.getOSSpecificHandle();
}
Expand Down
6 changes: 3 additions & 3 deletions lib/Interpreter/Paths.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ std::string NormalizePath(const std::string& Path);
///
/// \returns the library handle
///
void* DLOpen(const std::string& Path, std::string* Err = nullptr);
void* DLOpen(const std::string& Path, std::string& Err);

void* DLSym(const std::string& Name, std::string* Err = nullptr);
void* DLSym(const std::string& Name, std::string& Err);

///\brief Close a handle to a shared library.
///
Expand All @@ -58,7 +58,7 @@ void* DLSym(const std::string& Name, std::string* Err = nullptr);
///
/// \returns the library handle
///
void DLClose(void* Lib, std::string* Err = nullptr);
void DLClose(void* Lib, std::string& Err);
} // namespace platform

///\brief Replace all $TOKENS in a string with environent variable values.
Expand Down
9 changes: 9 additions & 0 deletions unittests/CppInterOp/DynamicLibraryManagerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"

#include "../../lib/Interpreter/Paths.h"

// This function isn't referenced outside its translation unit, but it
// can't use the "static" keyword because its address is used for
// GetMainExecutable (since some platforms don't support taking the
Expand Down Expand Up @@ -54,3 +56,10 @@ TEST(DynamicLibraryManagerTest, Sanity) {
// invalidated...
// EXPECT_FALSE(Cpp::GetFunctionAddress("ret_zero"));
}

TEST(UtilsPlatform, DLTest) {
std::string err_ = "";
auto dlopen_handle = Cpp::utils::platform::DLOpen(
"./unittests/CppInterOp/libTestSharedLib.dylib", err_);
(void)dlopen_handle;
}

0 comments on commit f6d467d

Please sign in to comment.