Skip to content

Commit 159c602

Browse files
authored
[UR] Refactor logging processing code (#20097)
Valgrind reports issues with the logger code that manipulates strings.
1 parent f5476db commit 159c602

File tree

3 files changed

+47
-33
lines changed

3 files changed

+47
-33
lines changed

unified-runtime/source/common/logger/ur_sinks.hpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,16 @@ class Sink {
3535
<< "[" << level_to_str(level) << "]: ";
3636
}
3737

38-
format(buffer, filename, lineno, fmt, std::forward<Args &&>(args)...);
38+
format(buffer, filename, lineno, fmt, std::forward<Args>(args)...);
3939
if (add_fileline) {
4040
buffer << " <" << filename << ":" << lineno << ">";
4141
}
4242
if (!skip_linebreak) {
4343
buffer << "\n";
4444
}
45+
46+
std::string message = buffer.str();
47+
4548
// This is a temporary workaround on windows, where UR adapter is teardowned
4649
// before the UR loader, which will result in access violation when we use print
4750
// function as the overrided print function was already released with the UR
@@ -50,12 +53,12 @@ class Sink {
5053
// using thier own sink class that inherit from logger::Sink.
5154
#if defined(_WIN32)
5255
if (isTearDowned) {
53-
std::cerr << buffer.str() << "\n";
56+
std::cerr << message << "\n";
5457
} else {
55-
print(level, buffer.str());
58+
print(level, message);
5659
}
5760
#else
58-
print(level, buffer.str());
61+
print(level, message);
5962
#endif
6063
}
6164

@@ -155,7 +158,7 @@ class Sink {
155158
}
156159
}
157160

158-
format(buffer, filename, lineno, ++fmt, std::forward<Args &&>(args)...);
161+
format(buffer, filename, lineno, ++fmt, std::forward<Args>(args)...);
159162
}
160163
};
161164

unified-runtime/source/common/ur_util.hpp

Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -167,14 +167,14 @@ getenv_to_vec(const char *env_var_name) {
167167
return std::nullopt;
168168
}
169169

170-
auto is_quoted = [](std::string &str) {
171-
return (str.front() == '\'' && str.back() == '\'') ||
172-
(str.front() == '"' && str.back() == '"');
170+
auto is_quoted = [](const std::string &str) {
171+
return str.size() >= 2 && ((str.front() == '\'' && str.back() == '\'') ||
172+
(str.front() == '"' && str.back() == '"'));
173173
};
174-
auto has_colon = [](std::string &str) {
174+
auto has_colon = [](const std::string &str) {
175175
return str.find(':') != std::string::npos;
176176
};
177-
auto has_semicolon = [](std::string &str) {
177+
auto has_semicolon = [](const std::string &str) {
178178
return str.find(';') != std::string::npos;
179179
};
180180

@@ -188,8 +188,7 @@ getenv_to_vec(const char *env_var_name) {
188188
}
189189

190190
if (is_quoted(value)) {
191-
value.erase(value.cbegin());
192-
value.erase(value.cend() - 1);
191+
value = value.substr(1, value.length() - 2);
193192
}
194193

195194
values_vec.push_back(value);
@@ -238,27 +237,29 @@ inline std::optional<EnvVarMap> getenv_to_map(const char *env_var_name,
238237
return std::nullopt;
239238
}
240239

241-
auto is_quoted = [](std::string &str) {
242-
return (str.front() == '\'' && str.back() == '\'') ||
243-
(str.front() == '"' && str.back() == '"');
240+
auto is_quoted = [](const std::string &str) {
241+
return str.size() >= 2 && ((str.front() == '\'' && str.back() == '\'') ||
242+
(str.front() == '"' && str.back() == '"'));
244243
};
245-
auto has_colon = [](std::string &str) {
244+
auto has_colon = [](const std::string &str) {
246245
return str.find(':') != std::string::npos;
247246
};
248247

249248
std::stringstream ss(*env_var);
250249
std::string key_value;
251250
while (std::getline(ss, key_value, main_delim)) {
252-
std::string key;
253-
std::string values;
254-
std::stringstream kv_ss(key_value);
255-
256251
if (reject_empty && !has_colon(key_value)) {
257252
throw_wrong_format_map(env_var_name, *env_var);
258253
}
259254

260-
std::getline(kv_ss, key, key_value_delim);
261-
std::getline(kv_ss, values);
255+
size_t colon_pos = key_value.find(key_value_delim);
256+
if (colon_pos == std::string::npos) {
257+
throw_wrong_format_map(env_var_name, *env_var);
258+
}
259+
260+
std::string key = key_value.substr(0, colon_pos);
261+
std::string values = key_value.substr(colon_pos + 1);
262+
262263
if (key.empty() || (reject_empty && values.empty()) ||
263264
(map.find(key) != map.end() && !allow_duplicate)) {
264265
throw_wrong_format_map(env_var_name, *env_var);
@@ -269,18 +270,28 @@ inline std::optional<EnvVarMap> getenv_to_map(const char *env_var_name,
269270
}
270271

271272
std::vector<std::string> values_vec;
272-
std::stringstream values_ss(values);
273-
std::string value;
274-
while (std::getline(values_ss, value, values_delim)) {
273+
274+
size_t start = 0;
275+
size_t pos = 0;
276+
while ((pos = values.find(values_delim, start)) != std::string::npos ||
277+
start < values.length()) {
278+
// No delimiter found, process the last value
279+
if (pos == std::string::npos) {
280+
pos = values.length();
281+
}
282+
283+
std::string value = values.substr(start, pos - start);
275284
if (value.empty() || (has_colon(value) && !is_quoted(value))) {
276285
throw_wrong_format_map(env_var_name, *env_var);
277286
}
278287
if (is_quoted(value)) {
279-
value.erase(value.cbegin());
280-
value.pop_back();
288+
value = value.substr(1, value.length() - 2);
281289
}
282-
values_vec.push_back(value);
290+
values_vec.push_back(std::move(value));
291+
292+
start = pos + 1;
283293
}
294+
284295
if (map.find(key) != map.end()) {
285296
map[key].insert(map[key].end(), values_vec.begin(), values_vec.end());
286297
} else {

unified-runtime/source/loader/ur_lib.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,14 @@ context_t::context_t() { parseEnvEnabledLayers(); }
3838
context_t::~context_t() {}
3939

4040
void context_t::parseEnvEnabledLayers() {
41-
auto maybeEnableEnvVarMap = getenv_to_map("UR_ENABLE_LAYERS", false);
42-
if (!maybeEnableEnvVarMap.has_value()) {
41+
auto maybeEnableEnvVarVec = getenv_to_vec("UR_ENABLE_LAYERS");
42+
if (!maybeEnableEnvVarVec.has_value()) {
4343
return;
4444
}
45-
auto enableEnvVarMap = maybeEnableEnvVarMap.value();
45+
auto enableEnvVarVec = maybeEnableEnvVarVec.value();
4646

47-
for (auto &key : enableEnvVarMap) {
48-
enabledLayerNames.insert(key.first);
47+
for (auto &layer : enableEnvVarVec) {
48+
enabledLayerNames.insert(layer);
4949
}
5050
}
5151

0 commit comments

Comments
 (0)