[break] to_substr(std::string&) now always points at the first element

changelog/current.md

@@ -8,6 +8,11 @@
   +bool csubstr::operator== (std::nullptr_t) const noexcept { return str == nullptr; }
   +bool csubstr::operator!= (std::nullptr_t) const noexcept { return str != nullptr; }
   ```
+- `to_substr(std::string &s)` and `to_csubstr(std::string const& s)` now point at the first element when the string is empty ([rapidyaml#264](https://github.com/biojppm/rapidyaml/pull/264#issuecomment-1264421024)):
+  ```diff
+  -    return c4::substr(!s.empty() ? &s[0] : nullptr, s.size());
+  +    return c4::substr(&s[0], s.size());
+  ```
 
 ### New features
 

src/c4/std/string.hpp

@@ -13,18 +13,32 @@ namespace c4 {
 
 //-----------------------------------------------------------------------------
 
-/** get a writeable view to an existing std::string */
-inline c4::substr to_substr(std::string &s)
+/** get a writeable view to an existing std::string.
+ * When the string is empty, the returned view will be pointing
+ * at the character with value '\0', but the size will be zero.
+ * @see https://en.cppreference.com/w/cpp/string/basic_string/operator_at
+ */
+C4_ALWAYS_INLINE c4::substr to_substr(std::string &s) noexcept
 {
-    char* data = ! s.empty() ? &s[0] : nullptr;
-    return c4::substr(data, s.size());
+    #if C4_CPP < 11
+    #error this function will do undefined behavior
+    #endif
+    // since c++11 it is legal to call s[s.size()].
+    return c4::substr(&s[0], s.size());
 }
 
-/** get a readonly view to an existing std::string */
-inline c4::csubstr to_csubstr(std::string const& s)
+/** get a readonly view to an existing std::string.
+ * When the string is empty, the returned view will be pointing
+ * at the character with value '\0', but the size will be zero.
+ * @see https://en.cppreference.com/w/cpp/string/basic_string/operator_at
+ */
+C4_ALWAYS_INLINE c4::csubstr to_csubstr(std::string const& s) noexcept
 {
-    const char* data = ! s.empty() ? &s[0] : nullptr;
-    return c4::csubstr(data, s.size());
+    #if C4_CPP < 11
+    #error this function will do undefined behavior
+    #endif
+    // since c++11 it is legal to call s[s.size()].
+    return c4::csubstr(&s[0], s.size());
 }
 
 //-----------------------------------------------------------------------------
@@ -50,7 +64,15 @@ inline size_t to_chars(c4::substr buf, std::string const& s)
 {
     C4_ASSERT(!buf.overlaps(to_csubstr(s)));
     size_t len = buf.len < s.size() ? buf.len : s.size();
-    memcpy(buf.str, s.data(), len);
+    // calling memcpy with null strings is undefined behavior
+    // and will wreak havoc in calling code's branches.
+    // see https://github.com/biojppm/rapidyaml/pull/264#issuecomment-1262133637
+    if(len)
+    {
+        C4_ASSERT(s.data() != nullptr);
+        C4_ASSERT(buf.str != nullptr);
+        memcpy(buf.str, s.data(), len);
+    }
     return s.size(); // return the number of needed chars
 }
 
@@ -59,7 +81,14 @@ inline bool from_chars(c4::csubstr buf, std::string * s)
 {
     s->resize(buf.len);
     C4_ASSERT(!buf.overlaps(to_csubstr(*s)));
-    memcpy(&(*s)[0], buf.str, buf.len);
+    // calling memcpy with null strings is undefined behavior
+    // and will wreak havoc in calling code's branches.
+    // see https://github.com/biojppm/rapidyaml/pull/264#issuecomment-1262133637
+    if(buf.len)
+    {
+        C4_ASSERT(buf.str != nullptr);
+        memcpy(&(*s)[0], buf.str, buf.len);
+    }
     return true;
 }
 

src/c4/std/string_fwd.hpp

@@ -30,8 +30,8 @@ using string = basic_string<char, char_traits<char>, allocator<char>>;
 
 namespace c4 {
 
-c4::substr to_substr(std::string &s);
-c4::csubstr to_csubstr(std::string const& s);
+C4_ALWAYS_INLINE c4::substr to_substr(std::string &s) noexcept;
+C4_ALWAYS_INLINE c4::csubstr to_csubstr(std::string const& s) noexcept;
 
 bool operator== (c4::csubstr ss, std::string const& s);
 bool operator!= (c4::csubstr ss, std::string const& s);