Skip to content

Commit

Permalink
Replace std::string_view usage with std::string (#115)
Browse files Browse the repository at this point in the history
A `std::string_view` doesn't guarantee null-terminated strings and thus cannot be safely used with UCX and UCXX loggers. Replace `std::string_view` with `std::string` to guarantee safe conversion.

Authors:
  - Peter Andreas Entschev (https://github.com/pentschev)

Approvers:
  - Mads R. B. Kristensen (https://github.com/madsbk)

URL: #115
  • Loading branch information
pentschev authored Nov 1, 2023
1 parent 457c461 commit 4464ca9
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 15 deletions.
13 changes: 6 additions & 7 deletions cpp/include/ucxx/delayed_submission.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include <functional>
#include <memory>
#include <mutex>
#include <string_view>
#include <string>
#include <utility>
#include <vector>

Expand Down Expand Up @@ -60,8 +60,7 @@ class DelayedSubmission {
template <typename T>
class BaseDelayedSubmissionCollection {
protected:
std::string_view _name{
"undefined"}; ///< The human-readable name of the collection, used for logging
std::string _name{"undefined"}; ///< The human-readable name of the collection, used for logging
bool _enabled{true}; ///< Whether the resource required to process the collection is enabled.
std::vector<T> _collection{}; ///< The collection.
std::mutex _mutex{}; ///< Mutex to provide access to `_collection`.
Expand Down Expand Up @@ -96,7 +95,7 @@ class BaseDelayedSubmissionCollection {
*
* @param[in] name human-readable name of the collection, used for logging.
*/
explicit BaseDelayedSubmissionCollection(const std::string_view name, const bool enabled)
explicit BaseDelayedSubmissionCollection(const std::string name, const bool enabled)
: _name{name}, _enabled{enabled}
{
}
Expand Down Expand Up @@ -150,7 +149,7 @@ class BaseDelayedSubmissionCollection {
}

if (itemsToProcess.size() > 0) {
ucxx_trace_req("Submitting %lu %s callbacks", itemsToProcess.size(), _name);
ucxx_trace_req("Submitting %lu %s callbacks", itemsToProcess.size(), _name.c_str());
for (auto& item : itemsToProcess)
processItem(item);
}
Expand All @@ -168,7 +167,7 @@ class RequestDelayedSubmissionCollection
std::pair<std::shared_ptr<Request>, DelayedSubmissionCallbackType> item) override;

public:
explicit RequestDelayedSubmissionCollection(const std::string_view name, const bool enabled);
explicit RequestDelayedSubmissionCollection(const std::string name, const bool enabled);
};

class GenericDelayedSubmissionCollection
Expand All @@ -179,7 +178,7 @@ class GenericDelayedSubmissionCollection
void processItem(DelayedSubmissionCallbackType callback) override;

public:
explicit GenericDelayedSubmissionCollection(const std::string_view name);
explicit GenericDelayedSubmissionCollection(const std::string name);
};

class DelayedSubmissionCollection {
Expand Down
12 changes: 6 additions & 6 deletions cpp/src/delayed_submission.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ DelayedSubmission::DelayedSubmission(const bool send,
{
}

RequestDelayedSubmissionCollection::RequestDelayedSubmissionCollection(const std::string_view name,
RequestDelayedSubmissionCollection::RequestDelayedSubmissionCollection(const std::string name,
const bool enabled)
: BaseDelayedSubmissionCollection<
std::pair<std::shared_ptr<Request>, DelayedSubmissionCallbackType>>{name, enabled}
Expand All @@ -32,7 +32,7 @@ RequestDelayedSubmissionCollection::RequestDelayedSubmissionCollection(const std
void RequestDelayedSubmissionCollection::scheduleLog(
std::pair<std::shared_ptr<Request>, DelayedSubmissionCallbackType> item)
{
ucxx_trace_req("Registered %s: %p", _name, item.first.get());
ucxx_trace_req("Registered %s: %p", _name.c_str(), item.first.get());
}

void RequestDelayedSubmissionCollection::processItem(
Expand All @@ -41,24 +41,24 @@ void RequestDelayedSubmissionCollection::processItem(
auto& req = item.first;
auto& callback = item.second;

ucxx_trace_req("Submitting %s callbacks: %p", _name, req.get());
ucxx_trace_req("Submitting %s callbacks: %p", _name.c_str(), req.get());

if (callback) callback();
}

GenericDelayedSubmissionCollection::GenericDelayedSubmissionCollection(const std::string_view name)
GenericDelayedSubmissionCollection::GenericDelayedSubmissionCollection(const std::string name)
: BaseDelayedSubmissionCollection<DelayedSubmissionCallbackType>{name, true}
{
}

void GenericDelayedSubmissionCollection::scheduleLog(DelayedSubmissionCallbackType item)
{
ucxx_trace_req("Registered %s", _name);
ucxx_trace_req("Registered %s", _name.c_str());
}

void GenericDelayedSubmissionCollection::processItem(DelayedSubmissionCallbackType callback)
{
ucxx_trace_req("Submitting %s callback", _name);
ucxx_trace_req("Submitting %s callback", _name.c_str());

if (callback) callback();
}
Expand Down
4 changes: 2 additions & 2 deletions cpp/src/utils/callback_notifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace utils {

#ifdef __GLIBC__
static const bool _useSpinlock = []() {
auto const libcVersion = std::string_view{gnu_get_libc_version()};
auto const libcVersion = std::string{gnu_get_libc_version()};
auto const dot = libcVersion.find(".");
if (dot == std::string::npos) {
return false;
Expand All @@ -27,7 +27,7 @@ static const bool _useSpinlock = []() {
auto const glibcMajor = std::stoi(libcVersion.substr(0, dot).data());
auto const glibcMinor = std::stoi(libcVersion.substr(dot + 1).data());
auto const use = glibcMajor < 2 || (glibcMajor == 2 && glibcMinor < 25);
ucxx_debug("glibc version %s detected, spinlock use is %d", libcVersion.data(), use);
ucxx_debug("glibc version %s detected, spinlock use is %d", libcVersion.c_str(), use);
return use;
}
}();
Expand Down

0 comments on commit 4464ca9

Please sign in to comment.