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 <[email protected]> Authored: Thu Jun 23 18:00:24 2016 -0400 Committer: Alan Conway <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
