You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ac...@apache.org on 2016/06/29 17:59:25 UTC

qpid-proton git commit: NO-JIRA: Add value_ref, clean access to pn_data_t* via value interface [Forced Update!]

Repository: qpid-proton
Updated Branches:
  refs/heads/master 919adfee1 -> 63bb49c94 (forced update)


NO-JIRA: Add value_ref, clean access to pn_data_t* via value interface

value_ref allows a class to return a "value&" that provides access to an
internally-held pn_data_t*, and provides a single point of "friendship" for
creating a proton::value by copying from a value_ref.


Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/63bb49c9
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/63bb49c9
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/63bb49c9

Branch: refs/heads/master
Commit: 63bb49c94e75a108e21bf514ab6b6da1b83c7665
Parents: 391a09f
Author: Alan Conway <ac...@redhat.com>
Authored: Thu Jun 23 18:00:24 2016 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Wed Jun 29 13:59:01 2016 -0400

----------------------------------------------------------------------
 .../bindings/cpp/include/proton/message.hpp     |  2 +-
 proton-c/bindings/cpp/include/proton/value.hpp  | 52 ++++++++++++++------
 proton-c/bindings/cpp/src/decoder.cpp           |  7 ++-
 proton-c/bindings/cpp/src/encoder.cpp           |  7 ++-
 proton-c/bindings/cpp/src/error_condition.cpp   |  2 +-
 proton-c/bindings/cpp/src/message.cpp           |  9 ++--
 proton-c/bindings/cpp/src/proton_bits.cpp       |  6 +--
 proton-c/bindings/cpp/src/terminus.cpp          |  3 +-
 proton-c/bindings/cpp/src/value.cpp             | 17 +++++--
 9 files changed, 64 insertions(+), 41 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/63bb49c9/proton-c/bindings/cpp/include/proton/message.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/include/proton/message.hpp b/proton-c/bindings/cpp/include/proton/message.hpp
index 4a562f7..3f683f6 100644
--- a/proton-c/bindings/cpp/include/proton/message.hpp
+++ b/proton-c/bindings/cpp/include/proton/message.hpp
@@ -323,7 +323,7 @@ class message {
     pn_message_t *pn_msg() const;
 
     mutable pn_message_t *pn_msg_;
-    mutable value body_;
+    mutable internal::value_ref body_;
     mutable property_map application_properties_;
     mutable annotation_map message_annotations_;
     mutable annotation_map delivery_annotations_;

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/63bb49c9/proton-c/bindings/cpp/include/proton/value.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/include/proton/value.hpp b/proton-c/bindings/cpp/include/proton/value.hpp
index 5438e00..6b4caca 100644
--- a/proton-c/bindings/cpp/include/proton/value.hpp
+++ b/proton-c/bindings/cpp/include/proton/value.hpp
@@ -32,15 +32,9 @@
 
 namespace proton {
 
-class message;
-
 namespace internal {
 
-class value_base;
-PN_CPP_EXTERN std::ostream& operator<<(std::ostream& o, const value_base& x);
-
-/// Separate value data from implicit conversion constructors to avoid
-/// recursions.
+/// Separate value data from implicit conversion constructors to avoid template recursion.
 class value_base {
   public:
     /// Get the type ID for the current value.
@@ -50,15 +44,13 @@ class value_base {
     PN_CPP_EXTERN bool empty() const;
 
   protected:
-    internal::data& data() const;
-    mutable class internal::data data_;
+    internal::data& data();
+    internal::data data_;
 
-    /// @cond INTERNAL    
-  friend class proton::message;
+  friend class value_ref;
   friend class codec::encoder;
   friend class codec::decoder;
   friend PN_CPP_EXTERN std::ostream& operator<<(std::ostream&, const value_base&);
-    /// @endcond
 };
 
 } // internal
@@ -116,13 +108,41 @@ class value : public internal::value_base, private internal::comparable<value> {
     /// @{
   friend PN_CPP_EXTERN bool operator==(const value& x, const value& y);
   friend PN_CPP_EXTERN bool operator<(const value& x, const value& y);
-    /// @}
+};
 
-    /// @cond INTERNAL
-    PN_CPP_EXTERN explicit value(const internal::data&);
-    /// @endcond
+namespace internal {
+
+// value_ref is a `pn_data_t* p` that can be returned as a value& and used to modify
+// the underlying value in-place.
+//
+// Classes with a value_ref member can return it as a value& in accessor functions.
+// It can also be used to copy a pn_data_t* p to a proton::value via: value(value_ref(p));
+// None of the constructors make copies, they just refer to the same value.
+//
+class value_ref : public value {
+  public:
+    value_ref(pn_data_t* = 0);
+    value_ref(const internal::data&);
+    value_ref(const value_base&);
+
+    // Use refer() not operator= to avoid confusion with value op=
+    void refer(pn_data_t*);
+    void refer(const internal::data&);
+    void refer(const value_base&);
+
+    // Reset to refer to nothing, release existing references. Equivalent to refer(0).
+    void reset();
+
+    // Assignments to value_ref means assigning to the value.
+    template <class T> value_ref& operator=(const T& x) {
+        static_cast<value&>(*this) = x;
+        return *this;
+    }
 };
 
+}
+
+
 /// @copydoc scalar::get
 /// @related proton::value
 template<class T> T get(const value& v) { T x; get(v, x); return x; }

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/63bb49c9/proton-c/bindings/cpp/src/decoder.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/decoder.cpp b/proton-c/bindings/cpp/src/decoder.cpp
index 1caa14a..9c941c8 100644
--- a/proton-c/bindings/cpp/src/decoder.cpp
+++ b/proton-c/bindings/cpp/src/decoder.cpp
@@ -42,8 +42,11 @@ namespace codec {
  * Note the pn_data_t "current" node is always pointing *before* the next value
  * to be returned by the decoder.
  */
-
-decoder::decoder(const internal::value_base& v, bool exact) : data(v.data()), exact_(exact) { rewind(); }
+decoder::decoder(const internal::value_base& v, bool exact)
+    : data(const_cast<internal::value_base&>(v).data()), exact_(exact)
+{
+    rewind();
+}
 
 namespace {
 template <class T> T check(T result) {

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/63bb49c9/proton-c/bindings/cpp/src/encoder.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/encoder.cpp b/proton-c/bindings/cpp/src/encoder.cpp
index 79f7826..e718c40 100644
--- a/proton-c/bindings/cpp/src/encoder.cpp
+++ b/proton-c/bindings/cpp/src/encoder.cpp
@@ -148,12 +148,11 @@ encoder& encoder::operator<<(const null&) { pn_data_put_null(pn_object()); retur
 encoder& encoder::operator<<(const internal::scalar_base& x) { return insert(x.atom_, pn_data_put_atom); }
 
 encoder& encoder::operator<<(const internal::value_base& x) {
-    if (*this == x.data_)
+    data d = x.data_;
+    if (*this == d)
         throw conversion_error("cannot insert into self");
-    if (x.empty()) {
+    if (!d || d.empty())
         return *this << null();
-    }
-    data d = x.data();
     d.rewind();
     check(append(d));
     return *this;

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/63bb49c9/proton-c/bindings/cpp/src/error_condition.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/error_condition.cpp b/proton-c/bindings/cpp/src/error_condition.cpp
index f200010..b927077 100644
--- a/proton-c/bindings/cpp/src/error_condition.cpp
+++ b/proton-c/bindings/cpp/src/error_condition.cpp
@@ -28,7 +28,7 @@ namespace proton {
 error_condition::error_condition(pn_condition_t* c) :
     name_(str(pn_condition_get_name(c))),
     description_(str(pn_condition_get_description(c))),
-    properties_(make_wrapper(pn_condition_info(c)))
+    properties_(internal::value_ref(pn_condition_info(c)))
 {}
 
 

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/63bb49c9/proton-c/bindings/cpp/src/message.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/message.cpp b/proton-c/bindings/cpp/src/message.cpp
index 6a05d90..ca5f2d5 100644
--- a/proton-c/bindings/cpp/src/message.cpp
+++ b/proton-c/bindings/cpp/src/message.cpp
@@ -54,7 +54,8 @@ message& message::operator=(message&& m) {
 message::message(const value& x) : pn_msg_(0) { body() = x; }
 
 message::~message() {
-    body_.data_ = internal::data();      // Must release body before pn_message_free
+    // Workaround proton bug: Must release all refs to body before calling pn_message_free()
+    body_.reset();
     pn_message_free(pn_msg_);
 }
 
@@ -69,7 +70,7 @@ void swap(message& x, message& y) {
 
 pn_message_t *message::pn_msg() const {
     if (!pn_msg_) pn_msg_ = pn_message();
-    body_.data_ = make_wrapper(pn_message_body(pn_msg_));
+    body_.refer(pn_message_body(pn_msg_));
     return pn_msg_;
 }
 
@@ -142,9 +143,7 @@ std::string message::reply_to() const {
 }
 
 void message::correlation_id(const message_id& id) {
-    value v;
-    v.data_ = make_wrapper(pn_message_correlation_id(pn_msg()));
-    v = id;
+    internal::value_ref(pn_message_correlation_id(pn_msg())) = id;
 }
 
 message_id message::correlation_id() const {

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/63bb49c9/proton-c/bindings/cpp/src/proton_bits.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/proton_bits.cpp b/proton-c/bindings/cpp/src/proton_bits.cpp
index 5514ebc..11dfd47 100644
--- a/proton-c/bindings/cpp/src/proton_bits.cpp
+++ b/proton-c/bindings/cpp/src/proton_bits.cpp
@@ -71,11 +71,7 @@ void set_error_condition(const error_condition& e, pn_condition_t *c) {
     if (!e.description().empty()) {
         pn_condition_set_description(c, e.description().c_str());
     }
-    // FIXME aconway 2016-05-09: value ref/value factory fix.
-    // TODO: This is wrong as it copies the value so doesn't change
-    // The internals of c
-    //proton::value v(pn_condition_info(c));
-    //v = e.properties();
+    internal::value_ref(pn_condition_info(c)) = e.properties();
 }
 
 }

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/63bb49c9/proton-c/bindings/cpp/src/terminus.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/terminus.cpp b/proton-c/bindings/cpp/src/terminus.cpp
index f378fdb..2c8da03 100644
--- a/proton-c/bindings/cpp/src/terminus.cpp
+++ b/proton-c/bindings/cpp/src/terminus.cpp
@@ -49,8 +49,7 @@ bool terminus::dynamic() const {
 }
 
 value terminus::node_properties() const {
-    value x(make_wrapper(pn_terminus_properties(object_)));
-    return x;
+    return internal::value_ref(pn_terminus_properties(object_));
 }
 
 }

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/63bb49c9/proton-c/bindings/cpp/src/value.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/value.cpp b/proton-c/bindings/cpp/src/value.cpp
index d0ef2ee..7ae59ff 100644
--- a/proton-c/bindings/cpp/src/value.cpp
+++ b/proton-c/bindings/cpp/src/value.cpp
@@ -34,7 +34,6 @@ using codec::start;
 
 value::value() {}
 value::value(const value& x) { *this = x; }
-value::value(const internal::data& x) { if (!x.empty()) data().copy(x); }
 #if PN_CPP_HAS_RVALUE_REFERENCES
 value::value(value&& x) { swap(*this, x); }
 value& value::operator=(value&& x) { swap(*this, x); return *this; }
@@ -45,7 +44,7 @@ value& value::operator=(const value& x) {
         if (x.empty())
             clear();
         else
-            data().copy(x.data());
+            data().copy(x.data_);
     }
     return *this;
 }
@@ -63,7 +62,7 @@ type_id value_base::type() const {
 bool value_base::empty() const { return type() == NULL_TYPE; }
 
 // On demand
-internal::data& value_base::data() const {
+internal::data& value_base::data() {
     if (!data_)
         data_ = internal::data::create();
     return data_;
@@ -194,6 +193,14 @@ std::ostream& operator<<(std::ostream& o, const internal::value_base& x) {
         return o << d;
     }
 }
-}
 
-}
+value_ref::value_ref(pn_data_t* p) { refer(p); }
+value_ref::value_ref(const internal::data& d) { refer(d); }
+value_ref::value_ref(const value_base& v) { refer(v); }
+
+void value_ref::refer(pn_data_t* p) { data_ = make_wrapper(p); }
+void value_ref::refer(const internal::data& d) { data_ = d; }
+void value_ref::refer(const value_base& v) { data_ = v.data_; }
+
+void value_ref::reset() { refer(0); }
+}}


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org