From 1b978596b575bda096a9e896a5cc3502943e5a60 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Sat, 26 Feb 2022 02:30:11 +0100
Subject: [PATCH 1/3] Value::mkString: Avoid crash from null string_view

---
 src/libexpr/eval.cc | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

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()));
+    }
 }
 
 

From bbf55383e7a9914cf79b8879e234f444cc774bfa Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Sat, 26 Feb 2022 02:30:29 +0100
Subject: [PATCH 2/3] Value::mkPath: Avoid potential crash from null
 string_view

---
 src/libexpr/eval.cc | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

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()));
+    }
 }
 
 

From da260f579d5f4a66d6d0d0e1297b69666d049d03 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Sun, 27 Feb 2022 12:50:18 +0100
Subject: [PATCH 3/3] 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(-)

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));
 }