From 1b978596b575bda096a9e896a5cc3502943e5a60 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 26 Feb 2022 02:30:11 +0100 Subject: Value::mkString: Avoid crash from null string_view --- src/libexpr/eval.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'src/libexpr/eval.cc') diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 2d7309738..b954e7bd9 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -805,7 +805,13 @@ LocalNoInline(void addErrorTrace(Error & e, const Pos & pos, const char * s, con void Value::mkString(std::string_view s) { - mkString(dupStringWithLen(s.data(), s.size())); + if (s.size() == 0) { + // s.data() may not be valid and we don't need to allocate. + mkString(""); + } + else { + mkString(dupStringWithLen(s.data(), s.size())); + } } -- cgit v1.2.3 From bbf55383e7a9914cf79b8879e234f444cc774bfa Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 26 Feb 2022 02:30:29 +0100 Subject: Value::mkPath: Avoid potential crash from null string_view --- src/libexpr/eval.cc | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'src/libexpr/eval.cc') diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index b954e7bd9..7f82aa5f4 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -842,7 +842,14 @@ void Value::mkStringMove(const char * s, const PathSet & context) void Value::mkPath(std::string_view s) { - mkPath(dupStringWithLen(s.data(), s.size())); + if (s.size() == 0) { + // Pathological, but better than crashing in dupStringWithLen, as + // s.data() may be null. + mkPath(""); + } + else { + mkPath(dupStringWithLen(s.data(), s.size())); + } } -- cgit v1.2.3 From da260f579d5f4a66d6d0d0e1297b69666d049d03 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 27 Feb 2022 12:50:18 +0100 Subject: dupStringWithLen -> makeImmutableString Refactor the `size == 0` logic into a new helper function that replaces dupStringWithLen. The name had to change, because unlike a `dup`-function, it does not always allocate a new string. --- src/libexpr/eval.cc | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) (limited to 'src/libexpr/eval.cc') diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 7f82aa5f4..5bf161cc0 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -63,9 +63,15 @@ static char * dupString(const char * s) } -static char * dupStringWithLen(const char * s, size_t size) +// When there's no need to write to the string, we can optimize away empty +// string allocations. +// This function handles makeImmutableStringWithLen(null, 0) by returning the +// empty string. +static const char * makeImmutableStringWithLen(const char * s, size_t size) { char * t; + if (size == 0) + return ""; #if HAVE_BOEHMGC t = GC_STRNDUP(s, size); #else @@ -75,6 +81,10 @@ static char * dupStringWithLen(const char * s, size_t size) return t; } +static inline const char * makeImmutableString(std::string_view s) { + return makeImmutableStringWithLen(s.data(), s.size()); +} + RootValue allocRootValue(Value * v) { @@ -805,13 +815,7 @@ LocalNoInline(void addErrorTrace(Error & e, const Pos & pos, const char * s, con void Value::mkString(std::string_view s) { - if (s.size() == 0) { - // s.data() may not be valid and we don't need to allocate. - mkString(""); - } - else { - mkString(dupStringWithLen(s.data(), s.size())); - } + mkString(makeImmutableString(s)); } @@ -842,14 +846,7 @@ void Value::mkStringMove(const char * s, const PathSet & context) void Value::mkPath(std::string_view s) { - if (s.size() == 0) { - // Pathological, but better than crashing in dupStringWithLen, as - // s.data() may be null. - mkPath(""); - } - else { - mkPath(dupStringWithLen(s.data(), s.size())); - } + mkPath(makeImmutableString(s)); } -- cgit v1.2.3