From 8e48b79db72939d8ded148400c7f0572c6882ef9 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Thu, 23 Jul 2015 17:46:30 -0700 Subject: [PATCH] Delete functions that return a pointer when the dynamic object is a rvalue. Summary: This diff is not yet complete, I want to see the contbuild before I change the functions that return references to member functions. It is unsafe to return a pointer when the dynamic object is a rvalue, because if the pointer escapes the expression after the object is destroyed, we go into segfault / undefined behavior land. I have deleted these overloads. The amount of valid code that is now disallowed is minimal. The only valid case I can think of is returing a pointer and passing it to a function in the same expression that does not save the pointer. However, this case is also dangerous, because if the function you pass it to decides to save the pointer for later, we are in trouble, e.g. save_ptr(dynamic("str").c_str()) Since there are simple workarounds (naming the object), I think that is a small price to pay for the greatly increased safety. The next step is to overload all members that return a reference to a member to move the member out if the dynamic is a rvalue: const dynamic& at(dynamic const&) const&; dynamic& at(dynamic const&) &; dynamic at(dynamic const&) &&; // Move out I also need to go over the code more carefully to make sure that nothing went wrong. Reviewed By: @marcinpe Differential Revision: D2257914 --- folly/dynamic-inl.h | 10 +++++----- folly/dynamic.cpp | 2 +- folly/dynamic.h | 16 ++++++++++------ 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/folly/dynamic-inl.h b/folly/dynamic-inl.h index 6db141be..6b42fab2 100644 --- a/folly/dynamic-inl.h +++ b/folly/dynamic-inl.h @@ -394,8 +394,8 @@ inline double& dynamic::getDouble() { return get(); } inline int64_t& dynamic::getInt() { return get(); } inline bool& dynamic::getBool() { return get(); } -inline const char* dynamic::data() const { return get().data(); } -inline const char* dynamic::c_str() const { return get().c_str(); } +inline const char* dynamic::data() const& { return get().data(); } +inline const char* dynamic::c_str() const& { return get().c_str(); } inline StringPiece dynamic::stringPiece() const { return get(); } template @@ -470,7 +470,7 @@ template inline dynamic& dynamic::setDefault(K&& k, V&& v) { std::forward(v))).first->second; } -inline dynamic* dynamic::get_ptr(dynamic const& idx) { +inline dynamic* dynamic::get_ptr(dynamic const& idx) & { return const_cast(const_cast(this)->get_ptr(idx)); } @@ -597,7 +597,7 @@ T dynamic::asImpl() const { // Return a T* to our type, or null if we're not that type. template -T* dynamic::get_nothrow() noexcept { +T* dynamic::get_nothrow() & noexcept { if (type_ != TypeInfo::type) { return nullptr; } @@ -605,7 +605,7 @@ T* dynamic::get_nothrow() noexcept { } template -T const* dynamic::get_nothrow() const noexcept { +T const* dynamic::get_nothrow() const& noexcept { return const_cast(this)->get_nothrow(); } diff --git a/folly/dynamic.cpp b/folly/dynamic.cpp index e5829244..f7be57e8 100644 --- a/folly/dynamic.cpp +++ b/folly/dynamic.cpp @@ -160,7 +160,7 @@ dynamic&& dynamic::getDefault(const dynamic& k, dynamic&& v) const { return std::move(v); } -const dynamic* dynamic::get_ptr(dynamic const& idx) const { +const dynamic* dynamic::get_ptr(dynamic const& idx) const& { if (auto* parray = get_nothrow()) { if (!idx.isInt()) { throw TypeError("int64", idx.type()); diff --git a/folly/dynamic.h b/folly/dynamic.h index b0444d9b..5bd36f4c 100644 --- a/folly/dynamic.h +++ b/folly/dynamic.h @@ -298,8 +298,10 @@ public: * * These will throw a TypeError if the dynamic is not a string. */ - const char* data() const; - const char* c_str() const; + const char* data() const&; + const char* data() && = delete; + const char* c_str() const&; + const char* c_str() && = delete; StringPiece stringPiece() const; /* @@ -374,8 +376,9 @@ public: * Using these with dynamic objects that are not arrays or objects * will throw a TypeError. */ - const dynamic* get_ptr(dynamic const&) const; - dynamic* get_ptr(dynamic const&); + const dynamic* get_ptr(dynamic const&) const&; + dynamic* get_ptr(dynamic const&) &; + dynamic* get_ptr(dynamic const&) && = delete; /* * This works for access to both objects and arrays. @@ -494,8 +497,9 @@ private: template T const& get() const; template T& get(); - template T* get_nothrow() noexcept; - template T const* get_nothrow() const noexcept; + template T* get_nothrow() & noexcept; + template T const* get_nothrow() const& noexcept; + template T* get_nothrow() && noexcept = delete; template T* getAddress() noexcept; template T const* getAddress() const noexcept; -- 2.34.1