From 4c0c8e5428b9fe47544897877a67f1e839b6223d Mon Sep 17 00:00:00 2001 From: Philipp Otterbein Date: Fri, 4 Oct 2024 01:24:39 +0200 Subject: [PATCH 1/5] cleanup: remove superfluous std::string copies --- src/libcmd/repl.cc | 2 +- src/libexpr/eval.cc | 13 ++++--------- src/libexpr/get-drvs.cc | 9 +++++---- src/libexpr/primops.cc | 4 ++-- src/libexpr/primops/fromTOML.cc | 2 +- src/libexpr/print.cc | 2 +- src/nix-build/nix-build.cc | 4 ++-- src/nix/config-check.cc | 12 ++++++------ 8 files changed, 22 insertions(+), 26 deletions(-) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 940b16dfd..1ae126e1e 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -654,7 +654,7 @@ ProcessLineResult NixRepl::processLine(std::string line) ss << "No documentation found.\n\n"; } - auto markdown = ss.str(); + auto markdown = ss.view(); logger->cout(trim(renderMarkdownToTerminal(markdown))); } else diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 9eae6078b..64516fd98 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -572,16 +572,13 @@ std::optional EvalState::getDoc(Value & v) s << docStr; s << '\0'; // for making a c string below - std::string ss = s.str(); return Doc { .pos = pos, .name = name, .arity = 0, // FIXME: figure out how deep by syntax only? It's not semantically useful though... .args = {}, - .doc = - // FIXME: this leaks; make the field std::string? - strdup(ss.data()), + .doc = makeImmutableString(s.view()), // NOTE: memory leak when compiled without GC }; } if (isFunctor(v)) { @@ -1805,11 +1802,9 @@ void ExprIf::eval(EvalState & state, Env & env, Value & v) void ExprAssert::eval(EvalState & state, Env & env, Value & v) { if (!state.evalBool(env, cond, pos, "in the condition of the assert statement")) { - auto exprStr = ({ - std::ostringstream out; - cond->show(state.symbols, out); - out.str(); - }); + std::ostringstream out; + cond->show(state.symbols, out); + auto exprStr = out.view(); if (auto eq = dynamic_cast(cond)) { try { diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index 20963ec91..1ac13fcd2 100644 --- a/src/libexpr/get-drvs.cc +++ b/src/libexpr/get-drvs.cc @@ -374,11 +374,12 @@ static void getDerivations(EvalState & state, Value & vIn, bound to the attribute with the "lower" name should take precedence). */ for (auto & i : v.attrs()->lexicographicOrder(state.symbols)) { + std::string_view symbol{state.symbols[i->name]}; try { - debug("evaluating attribute '%1%'", state.symbols[i->name]); - if (!std::regex_match(std::string(state.symbols[i->name]), attrRegex)) + debug("evaluating attribute '%1%'", symbol); + if (!std::regex_match(symbol.begin(), symbol.end(), attrRegex)) continue; - std::string pathPrefix2 = addToPath(pathPrefix, state.symbols[i->name]); + std::string pathPrefix2 = addToPath(pathPrefix, symbol); if (combineChannels) getDerivations(state, *i->value, pathPrefix2, autoArgs, drvs, done, ignoreAssertionFailures); else if (getDerivation(state, *i->value, pathPrefix2, drvs, done, ignoreAssertionFailures)) { @@ -392,7 +393,7 @@ static void getDerivations(EvalState & state, Value & vIn, } } } catch (Error & e) { - e.addTrace(state.positions[i->pos], "while evaluating the attribute '%s'", state.symbols[i->name]); + e.addTrace(state.positions[i->pos], "while evaluating the attribute '%s'", symbol); throw; } } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 7b6f222a8..4cc0de960 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -2129,7 +2129,7 @@ static void prim_toXML(EvalState & state, const PosIdx pos, Value * * args, Valu std::ostringstream out; NixStringContext context; printValueAsXML(state, true, false, *args[0], out, context, pos); - v.mkString(out.str(), context); + v.mkString(out.view(), context); } static RegisterPrimOp primop_toXML({ @@ -2237,7 +2237,7 @@ static void prim_toJSON(EvalState & state, const PosIdx pos, Value * * args, Val std::ostringstream out; NixStringContext context; printValueAsJSON(state, true, *args[0], pos, out, context); - v.mkString(out.str(), context); + v.mkString(out.view(), context); } static RegisterPrimOp primop_toJSON({ diff --git a/src/libexpr/primops/fromTOML.cc b/src/libexpr/primops/fromTOML.cc index b4f1df7a8..15568e529 100644 --- a/src/libexpr/primops/fromTOML.cc +++ b/src/libexpr/primops/fromTOML.cc @@ -66,7 +66,7 @@ static void prim_fromTOML(EvalState & state, const PosIdx pos, Value * * args, V attrs.alloc("_type").mkString("timestamp"); std::ostringstream s; s << t; - attrs.alloc("value").mkString(s.str()); + attrs.alloc("value").mkString(s.view()); v.mkAttrs(attrs); } else { throw std::runtime_error("Dates and times are not supported"); diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index 4d1a6868c..11a6c9161 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -460,7 +460,7 @@ private: std::ostringstream s; s << state.positions[v.payload.lambda.fun->pos]; - output << " @ " << filterANSIEscapes(s.str()); + output << " @ " << filterANSIEscapes(s.view()); } } else if (v.isPrimOp()) { if (v.primOp()) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index a5b9e1e54..f894d1034 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -260,9 +260,9 @@ static void main_nix_build(int argc, char * * argv) // read the shebang to understand which packages to read from. Since // this is handled via nix-shell -p, we wrap our ruby script execution // in ruby -e 'load' which ignores the shebangs. - envCommand = fmt("exec %1% %2% -e 'load(ARGV.shift)' -- %3% %4%", execArgs, interpreter, shellEscape(script), joined.str()); + envCommand = fmt("exec %1% %2% -e 'load(ARGV.shift)' -- %3% %4%", execArgs, interpreter, shellEscape(script), joined.view()); } else { - envCommand = fmt("exec %1% %2% %3% %4%", execArgs, interpreter, shellEscape(script), joined.str()); + envCommand = fmt("exec %1% %2% %3% %4%", execArgs, interpreter, shellEscape(script), joined.view()); } } diff --git a/src/nix/config-check.cc b/src/nix/config-check.cc index 6cf73785e..be252c3f1 100644 --- a/src/nix/config-check.cc +++ b/src/nix/config-check.cc @@ -26,17 +26,17 @@ std::string formatProtocol(unsigned int proto) return "unknown"; } -bool checkPass(const std::string & msg) { +bool checkPass(std::string_view msg) { notice(ANSI_GREEN "[PASS] " ANSI_NORMAL + msg); return true; } -bool checkFail(const std::string & msg) { +bool checkFail(std::string_view msg) { notice(ANSI_RED "[FAIL] " ANSI_NORMAL + msg); return false; } -void checkInfo(const std::string & msg) { +void checkInfo(std::string_view msg) { notice(ANSI_BLUE "[INFO] " ANSI_NORMAL + msg); } @@ -91,7 +91,7 @@ struct CmdConfigCheck : StoreCommand ss << "Multiple versions of nix found in PATH:\n"; for (auto & dir : dirs) ss << " " << dir << "\n"; - return checkFail(ss.str()); + return checkFail(ss.view()); } return checkPass("PATH contains only one nix version."); @@ -132,7 +132,7 @@ struct CmdConfigCheck : StoreCommand for (auto & dir : dirs) ss << " " << dir << "\n"; ss << "\n"; - return checkFail(ss.str()); + return checkFail(ss.view()); } return checkPass("All profiles are gcroots."); @@ -151,7 +151,7 @@ struct CmdConfigCheck : StoreCommand << "sync with the daemon.\n\n" << "Client protocol: " << formatProtocol(clientProto) << "\n" << "Store protocol: " << formatProtocol(storeProto) << "\n\n"; - return checkFail(ss.str()); + return checkFail(ss.view()); } return checkPass("Client protocol matches store protocol."); From caf3b5589160bea3de26958b71df78ca73979398 Mon Sep 17 00:00:00 2001 From: Philipp Otterbein Date: Mon, 7 Oct 2024 01:05:17 +0200 Subject: [PATCH 2/5] cont. cleanup: remove superfluous std::string copies --- src/libexpr/primops.cc | 15 +++++++++++---- src/libstore/daemon.cc | 4 ++-- src/libstore/outputs-spec.cc | 7 +++---- src/libutil/args.cc | 2 +- src/nix-build/nix-build.cc | 6 +++--- src/nix-env/user-env.cc | 4 +--- 6 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 4cc0de960..29121bb81 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -40,6 +40,13 @@ namespace nix { * Miscellaneous *************************************************************/ +static inline Value * mkString(EvalState & state, const std::csub_match & match) +{ + Value * v = state.allocValue(); + v->mkString({match.first, match.second}); + return v; +} + StringMap EvalState::realiseContext(const NixStringContext & context, StorePathSet * maybePathsOut, bool isIFD) { std::vector drvs; @@ -4268,7 +4275,7 @@ void prim_match(EvalState & state, const PosIdx pos, Value * * args, Value & v) if (!match[i + 1].matched) v2 = &state.vNull; else - (v2 = state.allocValue())->mkString(match[i + 1].str()); + v2 = mkString(state, match[i + 1]); v.mkList(list); } catch (std::regex_error & e) { @@ -4352,7 +4359,7 @@ void prim_split(EvalState & state, const PosIdx pos, Value * * args, Value & v) auto match = *i; // Add a string for non-matched characters. - (list[idx++] = state.allocValue())->mkString(match.prefix().str()); + list[idx++] = mkString(state, match.prefix()); // Add a list for matched substrings. const size_t slen = match.size() - 1; @@ -4363,14 +4370,14 @@ void prim_split(EvalState & state, const PosIdx pos, Value * * args, Value & v) if (!match[si + 1].matched) v2 = &state.vNull; else - (v2 = state.allocValue())->mkString(match[si + 1].str()); + v2 = mkString(state, match[si + 1]); } (list[idx++] = state.allocValue())->mkList(list2); // Add a string for non-matched suffix characters. if (idx == 2 * len) - (list[idx++] = state.allocValue())->mkString(match.suffix().str()); + list[idx++] = mkString(state, match.suffix()); } assert(idx == 2 * len + 1); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 6079eae7b..f0c3c866e 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -90,11 +90,11 @@ struct TunnelLogger : public Logger { if (ei.level > verbosity) return; - std::stringstream oss; + std::ostringstream oss; showErrorInfo(oss, ei, false); StringSink buf; - buf << STDERR_NEXT << oss.str(); + buf << STDERR_NEXT << oss.view(); enqueueMsg(buf.s); } diff --git a/src/libstore/outputs-spec.cc b/src/libstore/outputs-spec.cc index 86788a87e..f5ecbd74b 100644 --- a/src/libstore/outputs-spec.cc +++ b/src/libstore/outputs-spec.cc @@ -30,16 +30,15 @@ std::optional OutputsSpec::parseOpt(std::string_view s) { static std::regex regex(std::string { outputSpecRegexStr }); - std::smatch match; - std::string s2 { s }; // until some improves std::regex - if (!std::regex_match(s2, match, regex)) + std::cmatch match; + if (!std::regex_match(s.cbegin(), s.cend(), match, regex)) return std::nullopt; if (match[1].matched) return { OutputsSpec::All {} }; if (match[2].matched) - return OutputsSpec::Names { tokenizeString(match[2].str(), ",") }; + return OutputsSpec::Names { tokenizeString({match[2].first, match[2].second}, ",") }; assert(false); } diff --git a/src/libutil/args.cc b/src/libutil/args.cc index d58f4b4ae..4e87389d6 100644 --- a/src/libutil/args.cc +++ b/src/libutil/args.cc @@ -293,7 +293,7 @@ void RootArgs::parseCmdline(const Strings & _cmdline, bool allowShebang) // We match one space after `nix` so that we preserve indentation. // No space is necessary for an empty line. An empty line has basically no effect. if (std::regex_match(line, match, std::regex("^#!\\s*nix(:? |$)(.*)$"))) - shebangContent += match[2].str() + "\n"; + shebangContent += std::string_view{match[2].first, match[2].second} + "\n"; } for (const auto & word : parseShebangContent(shebangContent)) { cmdline.push_back(word); diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index f894d1034..8c52979f6 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -36,7 +36,7 @@ extern char * * environ __attribute__((weak)); /* Recreate the effect of the perl shellwords function, breaking up a * string into arguments like a shell word, including escapes */ -static std::vector shellwords(const std::string & s) +static std::vector shellwords(std::string_view s) { std::regex whitespace("^\\s+"); auto begin = s.cbegin(); @@ -51,7 +51,7 @@ static std::vector shellwords(const std::string & s) auto it = begin; for (; it != s.cend(); ++it) { if (st == sBegin) { - std::smatch match; + std::cmatch match; if (regex_search(it, s.cend(), match, whitespace)) { cur.append(begin, it); res.push_back(cur); @@ -173,7 +173,7 @@ static void main_nix_build(int argc, char * * argv) line = chomp(line); std::smatch match; if (std::regex_match(line, match, std::regex("^#!\\s*nix-shell\\s+(.*)$"))) - for (const auto & word : shellwords(match[1].str())) + for (const auto & word : shellwords({match[1].first, match[1].second})) args.push_back(word); } } diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc index a24dd11d6..ebd8ef42b 100644 --- a/src/nix-env/user-env.cc +++ b/src/nix-env/user-env.cc @@ -111,9 +111,7 @@ bool createUserEnv(EvalState & state, PackageInfos & elems, auto manifestFile = ({ std::ostringstream str; printAmbiguous(manifest, state.symbols, str, nullptr, std::numeric_limits::max()); - // TODO with C++20 we can use str.view() instead and avoid copy. - std::string str2 = str.str(); - StringSource source { str2 }; + StringSource source { str.view() }; state.store->addToStoreFromDump( source, "env-manifest.nix", FileSerialisationMethod::Flat, ContentAddressMethod::Raw::Text, HashAlgorithm::SHA256, references); }); From e21c7895ebec83db0ae39231fba9b35b080b6b1e Mon Sep 17 00:00:00 2001 From: Philipp Otterbein Date: Mon, 7 Oct 2024 02:05:53 +0200 Subject: [PATCH 3/5] MacOS built: add workaround for missing view() member of std::ostringstream --- src/libcmd/repl.cc | 4 ++-- src/libexpr/eval.cc | 6 +++--- src/libexpr/primops.cc | 4 ++-- src/libexpr/primops/fromTOML.cc | 2 +- src/libexpr/print.cc | 2 +- src/libstore/daemon.cc | 2 +- src/libutil/strings.cc | 15 +++++++++++++++ src/libutil/strings.hh | 5 +++++ src/nix-build/nix-build.cc | 4 ++-- src/nix-env/user-env.cc | 2 +- src/nix/config-check.cc | 12 ++++++------ 11 files changed, 39 insertions(+), 19 deletions(-) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 1ae126e1e..018171889 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -645,7 +645,7 @@ ProcessLineResult NixRepl::processLine(std::string line) logger->cout(trim(renderMarkdownToTerminal(markdown))); } else if (fallbackPos) { - std::stringstream ss; + std::ostringstream ss; ss << "Attribute `" << fallbackName << "`\n\n"; ss << " … defined at " << state->positions[fallbackPos] << "\n\n"; if (fallbackDoc) { @@ -654,7 +654,7 @@ ProcessLineResult NixRepl::processLine(std::string line) ss << "No documentation found.\n\n"; } - auto markdown = ss.view(); + auto markdown = toView(ss); logger->cout(trim(renderMarkdownToTerminal(markdown))); } else diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 64516fd98..8fbba7c14 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -539,7 +539,7 @@ std::optional EvalState::getDoc(Value & v) if (v.isLambda()) { auto exprLambda = v.payload.lambda.fun; - std::stringstream s(std::ios_base::out); + std::ostringstream s(std::ios_base::out); std::string name; auto pos = positions[exprLambda->getPos()]; std::string docStr; @@ -578,7 +578,7 @@ std::optional EvalState::getDoc(Value & v) .name = name, .arity = 0, // FIXME: figure out how deep by syntax only? It's not semantically useful though... .args = {}, - .doc = makeImmutableString(s.view()), // NOTE: memory leak when compiled without GC + .doc = makeImmutableString(toView(s)), // NOTE: memory leak when compiled without GC }; } if (isFunctor(v)) { @@ -1804,7 +1804,7 @@ void ExprAssert::eval(EvalState & state, Env & env, Value & v) if (!state.evalBool(env, cond, pos, "in the condition of the assert statement")) { std::ostringstream out; cond->show(state.symbols, out); - auto exprStr = out.view(); + auto exprStr = toView(out); if (auto eq = dynamic_cast(cond)) { try { diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 29121bb81..a3c8a0c9c 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -2136,7 +2136,7 @@ static void prim_toXML(EvalState & state, const PosIdx pos, Value * * args, Valu std::ostringstream out; NixStringContext context; printValueAsXML(state, true, false, *args[0], out, context, pos); - v.mkString(out.view(), context); + v.mkString(toView(out), context); } static RegisterPrimOp primop_toXML({ @@ -2244,7 +2244,7 @@ static void prim_toJSON(EvalState & state, const PosIdx pos, Value * * args, Val std::ostringstream out; NixStringContext context; printValueAsJSON(state, true, *args[0], pos, out, context); - v.mkString(out.view(), context); + v.mkString(toView(out), context); } static RegisterPrimOp primop_toJSON({ diff --git a/src/libexpr/primops/fromTOML.cc b/src/libexpr/primops/fromTOML.cc index 15568e529..264046711 100644 --- a/src/libexpr/primops/fromTOML.cc +++ b/src/libexpr/primops/fromTOML.cc @@ -66,7 +66,7 @@ static void prim_fromTOML(EvalState & state, const PosIdx pos, Value * * args, V attrs.alloc("_type").mkString("timestamp"); std::ostringstream s; s << t; - attrs.alloc("value").mkString(s.view()); + attrs.alloc("value").mkString(toView(s)); v.mkAttrs(attrs); } else { throw std::runtime_error("Dates and times are not supported"); diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index 11a6c9161..d62aaf25f 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -460,7 +460,7 @@ private: std::ostringstream s; s << state.positions[v.payload.lambda.fun->pos]; - output << " @ " << filterANSIEscapes(s.view()); + output << " @ " << filterANSIEscapes(toView(s)); } } else if (v.isPrimOp()) { if (v.primOp()) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index f0c3c866e..b921dbe2d 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -94,7 +94,7 @@ struct TunnelLogger : public Logger showErrorInfo(oss, ei, false); StringSink buf; - buf << STDERR_NEXT << oss.view(); + buf << STDERR_NEXT << toView(oss); enqueueMsg(buf.s); } diff --git a/src/libutil/strings.cc b/src/libutil/strings.cc index 5cad95758..d1c9f700c 100644 --- a/src/libutil/strings.cc +++ b/src/libutil/strings.cc @@ -6,6 +6,21 @@ namespace nix { +struct view_stringbuf : public std::stringbuf +{ + inline std::string_view toView() + { + auto begin = pbase(); + return {begin, begin + pubseekoff(0, std::ios_base::cur, std::ios_base::out)}; + } +}; + +std::string_view toView(const std::ostringstream & os) +{ + auto buf = static_cast(os.rdbuf()); + return buf->toView(); +} + template std::list tokenizeString(std::string_view s, std::string_view separators); template std::set tokenizeString(std::string_view s, std::string_view separators); template std::vector tokenizeString(std::string_view s, std::string_view separators); diff --git a/src/libutil/strings.hh b/src/libutil/strings.hh index 88b48d770..533126be1 100644 --- a/src/libutil/strings.hh +++ b/src/libutil/strings.hh @@ -8,6 +8,11 @@ namespace nix { +/* + * workaround for unavailable view() method (C++20) of std::ostringstream under MacOS with clang-16 + */ +std::string_view toView(const std::ostringstream & os); + /** * String tokenizer. * diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 8c52979f6..7d32a6f97 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -260,9 +260,9 @@ static void main_nix_build(int argc, char * * argv) // read the shebang to understand which packages to read from. Since // this is handled via nix-shell -p, we wrap our ruby script execution // in ruby -e 'load' which ignores the shebangs. - envCommand = fmt("exec %1% %2% -e 'load(ARGV.shift)' -- %3% %4%", execArgs, interpreter, shellEscape(script), joined.view()); + envCommand = fmt("exec %1% %2% -e 'load(ARGV.shift)' -- %3% %4%", execArgs, interpreter, shellEscape(script), toView(joined)); } else { - envCommand = fmt("exec %1% %2% %3% %4%", execArgs, interpreter, shellEscape(script), joined.view()); + envCommand = fmt("exec %1% %2% %3% %4%", execArgs, interpreter, shellEscape(script), toView(joined)); } } diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc index ebd8ef42b..ee62077c0 100644 --- a/src/nix-env/user-env.cc +++ b/src/nix-env/user-env.cc @@ -111,7 +111,7 @@ bool createUserEnv(EvalState & state, PackageInfos & elems, auto manifestFile = ({ std::ostringstream str; printAmbiguous(manifest, state.symbols, str, nullptr, std::numeric_limits::max()); - StringSource source { str.view() }; + StringSource source { toView(str) }; state.store->addToStoreFromDump( source, "env-manifest.nix", FileSerialisationMethod::Flat, ContentAddressMethod::Raw::Text, HashAlgorithm::SHA256, references); }); diff --git a/src/nix/config-check.cc b/src/nix/config-check.cc index be252c3f1..a72b06542 100644 --- a/src/nix/config-check.cc +++ b/src/nix/config-check.cc @@ -87,11 +87,11 @@ struct CmdConfigCheck : StoreCommand } if (dirs.size() != 1) { - std::stringstream ss; + std::ostringstream ss; ss << "Multiple versions of nix found in PATH:\n"; for (auto & dir : dirs) ss << " " << dir << "\n"; - return checkFail(ss.view()); + return checkFail(toView(ss)); } return checkPass("PATH contains only one nix version."); @@ -125,14 +125,14 @@ struct CmdConfigCheck : StoreCommand } if (!dirs.empty()) { - std::stringstream ss; + std::ostringstream ss; ss << "Found profiles outside of " << settings.nixStateDir << "/profiles.\n" << "The generation this profile points to might not have a gcroot and could be\n" << "garbage collected, resulting in broken symlinks.\n\n"; for (auto & dir : dirs) ss << " " << dir << "\n"; ss << "\n"; - return checkFail(ss.view()); + return checkFail(toView(ss)); } return checkPass("All profiles are gcroots."); @@ -145,13 +145,13 @@ struct CmdConfigCheck : StoreCommand : PROTOCOL_VERSION; if (clientProto != storeProto) { - std::stringstream ss; + std::ostringstream ss; ss << "Warning: protocol version of this client does not match the store.\n" << "While this is not necessarily a problem it's recommended to keep the client in\n" << "sync with the daemon.\n\n" << "Client protocol: " << formatProtocol(clientProto) << "\n" << "Store protocol: " << formatProtocol(storeProto) << "\n\n"; - return checkFail(ss.view()); + return checkFail(toView(ss)); } return checkPass("Client protocol matches store protocol."); From de96f632f8c68abfb80d7d45ba3ad0bc3f451bcd Mon Sep 17 00:00:00 2001 From: Philipp Otterbein Date: Tue, 8 Oct 2024 02:25:14 +0200 Subject: [PATCH 4/5] std::string_view shall not be null terminated --- src/libexpr/eval.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 8fbba7c14..f17753415 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -539,7 +539,7 @@ std::optional EvalState::getDoc(Value & v) if (v.isLambda()) { auto exprLambda = v.payload.lambda.fun; - std::ostringstream s(std::ios_base::out); + std::ostringstream s; std::string name; auto pos = positions[exprLambda->getPos()]; std::string docStr; @@ -571,8 +571,6 @@ std::optional EvalState::getDoc(Value & v) s << docStr; - s << '\0'; // for making a c string below - return Doc { .pos = pos, .name = name, From a353a99269a88bdf1f96a2512c2102b965b5f531 Mon Sep 17 00:00:00 2001 From: Philipp Otterbein Date: Tue, 8 Oct 2024 02:25:52 +0200 Subject: [PATCH 5/5] cont. cleanup: remove superfluous std::string copies --- src/libmain/progress-bar.cc | 4 ++-- src/libutil/logging.cc | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index e63d4f13f..22f890f7d 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -158,10 +158,10 @@ public: { auto state(state_.lock()); - std::stringstream oss; + std::ostringstream oss; showErrorInfo(oss, ei, loggerSettings.showTrace.get()); - log(*state, ei.level, oss.str()); + log(*state, ei.level, toView(oss)); } void log(State & state, Verbosity lvl, std::string_view s) diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 3ef71a716..3d7371457 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -85,10 +85,10 @@ public: void logEI(const ErrorInfo & ei) override { - std::stringstream oss; + std::ostringstream oss; showErrorInfo(oss, ei, loggerSettings.showTrace.get()); - log(ei.level, oss.str()); + log(ei.level, toView(oss)); } void startActivity(ActivityId act, Verbosity lvl, ActivityType type,