QPID-5489: Uuid code improvements

- Don't use uuid_compare() as it will get the wrong version of the
  function under FreeBSD which has a uuid library build into libc with
  different function signatures from libuuid but some overlapping names.
- Reorganise the uuid code to limit the used external symbols to uuid_generate(),
  uuid_parse(), uuid_unparse()
- Minimise the framing::Uuid code so that it is a simple wrapper around types::Uuid
- Use uuid_generate() as the symbol to search in CMake (uuid_compare() isn't used
  in qpid anymore).

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1559017 13f79535-47bb-0310-9956-ffa450edef68

12 files changed, 35 insertions, 183 deletions

diff --git a/qpid/cpp/src/CMakeLists.txt b/qpid/cpp/src/CMakeLists.txt
index cace78902b..25726f9793 100644
--- a/qpid/cpp/src/CMakeLists.txt
+++ b/qpid/cpp/src/CMakeLists.txt
@@ -383,7 +383,7 @@ if (NOT CMAKE_SYSTEM_NAME STREQUAL Windows)
   endif (NOT CLOCK_GETTIME_IN_LIBC)
 
   # Ensure we have uuid library
-  CHECK_LIBRARY_EXISTS (uuid uuid_compare "" HAVE_UUID)
+  CHECK_LIBRARY_EXISTS (uuid uuid_generate "" HAVE_UUID)
   CHECK_INCLUDE_FILES(uuid/uuid.h HAVE_UUID_H)
   if (NOT HAVE_UUID AND NOT HAVE_UUID_H)
     message(FATAL_ERROR "Uuid library and/or header file not found")
@@ -1006,7 +1006,7 @@ add_msvc_version (qpidclient library dll)
 
 add_library (qpidclient SHARED ${qpidclient_SOURCES})
 
-target_link_libraries (qpidclient qpidcommon
+target_link_libraries (qpidclient qpidcommon qpidtypes
                        ${Boost_PROGRAM_OPTIONS_LIBRARY}
                        ${ssl_LIBS})
 
diff --git a/qpid/cpp/src/qpid/broker/Selector.cpp b/qpid/cpp/src/qpid/broker/Selector.cpp
index 0c25e7338b..40a0f019ab 100644
--- a/qpid/cpp/src/qpid/broker/Selector.cpp
+++ b/qpid/cpp/src/qpid/broker/Selector.cpp
@@ -30,6 +30,7 @@
 #include "qpid/log/Statement.h"
 #include "qpid/types/Variant.h"
 
+#include <stdexcept>
 #include <string>
 #include <sstream>
 #include "qpid/sys/unordered_map.h"
diff --git a/qpid/cpp/src/qpid/broker/System.cpp b/qpid/cpp/src/qpid/broker/System.cpp
index f3b6f96d0c..f3535b0eec 100644
--- a/qpid/cpp/src/qpid/broker/System.cpp
+++ b/qpid/cpp/src/qpid/broker/System.cpp
@@ -64,7 +64,7 @@ System::System (string _dataDir, Broker* broker)
             }
         }
 
-        mgmtObject = _qmf::System::shared_ptr(new _qmf::System(agent, this, types::Uuid(systemId.c_array())));
+        mgmtObject = _qmf::System::shared_ptr(new _qmf::System(agent, this, systemId));
         qpid::sys::SystemInfo::getSystemId (osName,
                                             nodeName,
                                             release,
diff --git a/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp b/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp
index 98dd70dceb..c8a293e19e 100644
--- a/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp
+++ b/qpid/cpp/src/qpid/broker/amqp_0_10/MessageTransfer.cpp
@@ -79,7 +79,7 @@ amqp::MessageId MessageTransfer::getMessageId() const
 
     amqp::MessageId r;
     if (mp->hasMessageId()) {
-        r.set(amqp::CharSequence::create(&mp->getMessageId()[0],16), types::VAR_UUID);
+        r.set(amqp::CharSequence::create(mp->getMessageId().data(),16), types::VAR_UUID);
     }
     return r;
 }
diff --git a/qpid/cpp/src/qpid/framing/Uuid.cpp b/qpid/cpp/src/qpid/framing/Uuid.cpp
index e377c1172d..eb4c33be75 100644
--- a/qpid/cpp/src/qpid/framing/Uuid.cpp
+++ b/qpid/cpp/src/qpid/framing/Uuid.cpp
@@ -29,47 +29,13 @@ namespace framing {
 
 using namespace std;
 
-static const size_t UNPARSED_SIZE=36; 
+Uuid::Uuid(bool unique):
+  qpid::types::Uuid(unique)
+{}
 
-Uuid::Uuid(bool unique) {
-    if (unique) {
-        generate();
-    } else {
-        clear();
-    }
-}
-
-Uuid::Uuid(const uint8_t* data) {
-    assign(data);
-}
-
-Uuid::Uuid(const std::string& s) {
-    if (s.size() != UNPARSED_SIZE)
-        throw IllegalArgumentException(QPID_MSG("Invalid UUID: " << s));
-    if (uuid_parse(const_cast<char *>(&s[0]), c_array()) != 0)
-        throw IllegalArgumentException(QPID_MSG("Invalid UUID: " << s));
-}
-
-void Uuid::assign(const uint8_t* data) {
-    // This const cast is for Solaris which has a 
-    // uuid_copy that takes a non const 2nd argument
-    uuid_copy(c_array(), const_cast<uint8_t*>(data));
-}
-
-void Uuid::generate() {
-    uuid_generate(c_array());
-}
-
-void Uuid::clear() {
-    uuid_clear(c_array());
-}
-
-// Force int 0/!0 to false/true; avoids compile warnings.
-bool Uuid::isNull() const {
-    // This const cast is for Solaris which has a 
-    // uuid_is_null that takes a non const argument
-    return !!uuid_is_null(const_cast<uint8_t*>(data()));
-}
+Uuid::Uuid(const uint8_t* data):
+  qpid::types::Uuid(data)
+{}
 
 void Uuid::encode(Buffer& buf) const {
     buf.putRawData(data(), size());
@@ -78,29 +44,9 @@ void Uuid::encode(Buffer& buf) const {
 void Uuid::decode(Buffer& buf) {
     if (buf.available() < size())
         throw IllegalArgumentException(QPID_MSG("Not enough data for UUID."));
-    buf.getRawData(c_array(), size());
-}
-
-ostream& operator<<(ostream& out, Uuid uuid) {
-    char unparsed[UNPARSED_SIZE + 1];
-    uuid_unparse(uuid.data(), unparsed);
-    return out << unparsed;
-}
-
-istream& operator>>(istream& in, Uuid& uuid) {
-    char unparsed[UNPARSED_SIZE + 1] = {0};
-    in.get(unparsed, sizeof(unparsed));
-    if (!in.fail()) {
-        if (uuid_parse(unparsed, uuid.c_array()) != 0) 
-            in.setstate(ios::failbit);
-    }
-    return in;
-}
 
-std::string Uuid::str() const {
-    std::ostringstream os;
-    os << *this;
-    return os.str();
+    // Break qpid::types::Uuid encapsulation - Nasty, but efficient
+    buf.getRawData(const_cast<uint8_t*>(data()), size());
 }
 
 }} // namespace qpid::framing
diff --git a/qpid/cpp/src/qpid/framing/Uuid.h b/qpid/cpp/src/qpid/framing/Uuid.h
index e9e56ed7c9..906e20951f 100644
--- a/qpid/cpp/src/qpid/framing/Uuid.h
+++ b/qpid/cpp/src/qpid/framing/Uuid.h
@@ -22,7 +22,7 @@
 #include "qpid/CommonImportExport.h"
 #include "qpid/sys/IntegerTypes.h"
 
-#include <boost/array.hpp>
+#include "qpid/types/Uuid.h"
 
 #include <ostream>
 #include <istream>
@@ -33,62 +33,25 @@ namespace framing {
 class Buffer;
 
 /**
- * A UUID is represented as a boost::array of 16 bytes.
- *
- * Full value semantics, operators ==, < etc.  are provided by
- * boost::array so Uuid can be the key type in a map etc.
- *
- * TODO: change this implementation as it leaks boost into the
- * client API
+ * Framing UUID is now a thine wrapper around qpid::types::Uuid
  */
-struct Uuid : public boost::array<uint8_t, 16> {
+struct Uuid : public qpid::types::Uuid {
     /** If unique is true, generate a unique ID else a null ID. */
     QPID_COMMON_EXTERN Uuid(bool unique=false);
 
     /** Copy from 16 bytes of data. */
     QPID_COMMON_EXTERN Uuid(const uint8_t* data);
 
-    /** Parse format 1b4e28ba-2fa1-11d2-883f-b9a761bde3fb. */
-    QPID_COMMON_EXTERN Uuid(const std::string&);
-
-    // Default op= and copy ctor are fine.
-    // boost::array gives us ==, < etc.
-
-    /** Copy from 16 bytes of data. */
-    QPID_COMMON_EXTERN void assign(const uint8_t* data);
-
-    /** Set to a new unique identifier. */
-    QPID_COMMON_EXTERN void generate();
-
-    /** Set to all zeros. */
-    QPID_COMMON_EXTERN void clear();
-
-    /** Test for null (all zeros). */
-    QPID_COMMON_EXTERN bool isNull() const;
-    QPID_COMMON_INLINE_EXTERN operator bool() const { return !isNull(); }
-    QPID_COMMON_INLINE_EXTERN bool operator!() const { return isNull(); }
+    // We get most of our operations directly from qpid::types::Uuid
+    QPID_COMMON_INLINE_EXTERN static size_t size()
+        { return SIZE; }
 
     QPID_COMMON_EXTERN void encode(framing::Buffer& buf) const;
     QPID_COMMON_EXTERN void decode(framing::Buffer& buf);
     QPID_COMMON_INLINE_EXTERN uint32_t encodedSize() const
-        { return static_cast<uint32_t>(size()); }
-
-    /** String value in format 1b4e28ba-2fa1-11d2-883f-b9a761bde3fb. */
-    QPID_COMMON_EXTERN std::string str() const;
-
-    template <class S> void serialize(S& s) {
-        s.raw(begin(), size());
-    }
+        { return size(); }
 };
 
-/** Print in format 1b4e28ba-2fa1-11d2-883f-b9a761bde3fb. */
-QPID_COMMON_EXTERN std::ostream& operator<<(std::ostream&, Uuid);
-
-/** Read from format 1b4e28ba-2fa1-11d2-883f-b9a761bde3fb. */
-QPID_COMMON_EXTERN std::istream& operator>>(std::istream&, Uuid&);
-
 }} // namespace qpid::framing
 
-
-
 #endif  /*!QPID_FRAMING_UUID_H*/
diff --git a/qpid/cpp/src/qpid/ha/Primary.cpp b/qpid/cpp/src/qpid/ha/Primary.cpp
index b29a550559..496704f737 100644
--- a/qpid/cpp/src/qpid/ha/Primary.cpp
+++ b/qpid/cpp/src/qpid/ha/Primary.cpp
@@ -284,7 +284,7 @@ void Primary::exchangeCreate(const ExchangePtr& ex) {
         QPID_LOG(debug, logPrefix << "Created exchange " << ex->getName()
                  << " replication: " << printable(level));
          // Give each exchange a unique id to avoid confusion of same-named exchanges.
-        args.set(QPID_HA_UUID, FieldTable::ValuePtr(new UuidValue(&Uuid(true)[0])));
+        args.set(QPID_HA_UUID, FieldTable::ValuePtr(new UuidValue(Uuid(true).data())));
     }
     ex->setArgs(args);
 }
diff --git a/qpid/cpp/src/qpid/sys/windows/uuid.cpp b/qpid/cpp/src/qpid/sys/windows/uuid.cpp
index d4f57fcab6..c8804d9ee7 100644
--- a/qpid/cpp/src/qpid/sys/windows/uuid.cpp
+++ b/qpid/cpp/src/qpid/sys/windows/uuid.cpp
@@ -65,15 +65,6 @@ void printHex (const unsigned char *bytes, char *buf, int n) {
 } // namespace
 
 
-void uuid_clear (uuid_t uu) {
-    // all zeros, no change between GUID and UUID
-    UuidCreateNil (reinterpret_cast<UUID*>(uu));
-}
-
-void uuid_copy (uuid_t dst, const uuid_t src) {
-    memcpy (dst, src, qpid::sys::UuidSize);
-}
-
 void uuid_generate (uuid_t out) {
     UUID guid;
     UuidCreate (&guid);
@@ -81,11 +72,6 @@ void uuid_generate (uuid_t out) {
     toUuid (&guid, out);
 }
 
-int uuid_is_null (const uuid_t uu) {
-    RPC_STATUS unused;
-    return UuidIsNil ((UUID*)uu, &unused);
-}
-
 int uuid_parse (const char *in, uuid_t uu) {
     UUID guid;
     if (UuidFromString ((unsigned char*)in, &guid) != RPC_S_OK)
@@ -104,34 +90,3 @@ void uuid_unparse (const uuid_t uu, char *out) {
     printHex (in+10, out+24, 6);
     out[36] = '\0';
 }
-
-namespace {
-
-typedef struct {
-    uint32_t time_low;
-    uint16_t time_mid;
-    uint16_t time_hi_and_version;
-    uint8_t  clock_seq_hi_and_reserved;
-    uint8_t  clock_seq_low;
-    uint8_t  node[6];
-} rfc_uuid_t;
-
-#undef RFC_CMP
-#define RFC_CMP(a, b) if (a != b) return (a < b) ? -1 : 1
-
-}
-
-int uuid_compare (const uuid_t a, const uuid_t b) {
-    // Could convert each to a GUID and then use UuidEqual(),
-    // but RFC test is straight forward
-    rfc_uuid_t* u1 = (rfc_uuid_t *) a;
-    rfc_uuid_t* u2 = (rfc_uuid_t *) b;
-    RFC_CMP (u1->time_low, u2->time_low);
-    RFC_CMP (u1->time_mid, u2->time_mid);
-    RFC_CMP (u1->time_hi_and_version, u2->time_hi_and_version);
-    RFC_CMP (u1->clock_seq_hi_and_reserved, u2->clock_seq_hi_and_reserved);
-    RFC_CMP (u1->clock_seq_low, u2->clock_seq_low);
-    for (int i = 0; i < 6; i++)
-        RFC_CMP (u1->node[i], u2->node[i]);
-    return 0;
-}
diff --git a/qpid/cpp/src/qpid/sys/windows/uuid.h b/qpid/cpp/src/qpid/sys/windows/uuid.h
index 8ab132e9ce..69da709a03 100644
--- a/qpid/cpp/src/qpid/sys/windows/uuid.h
+++ b/qpid/cpp/src/qpid/sys/windows/uuid.h
@@ -28,12 +28,8 @@
 namespace qpid { namespace sys { const size_t UuidSize = 16; }}
 typedef uint8_t uuid_t[qpid::sys::UuidSize];
 
-QPID_TYPES_EXTERN void uuid_clear (uuid_t uu);
-QPID_TYPES_EXTERN void uuid_copy (uuid_t dst, const uuid_t src);
 QPID_TYPES_EXTERN void uuid_generate (uuid_t out);
-QPID_TYPES_EXTERN int  uuid_is_null (const uuid_t uu);          // Returns 1 if null, else 0
 QPID_TYPES_EXTERN int  uuid_parse (const char *in, uuid_t uu);  // Returns 0 on success, else -1
 QPID_TYPES_EXTERN void uuid_unparse (const uuid_t uu, char *out);
-QPID_TYPES_EXTERN int  uuid_compare (const uuid_t a, const uuid_t b);
 
 #endif  /*!_sys_windows_uuid_h*/
diff --git a/qpid/cpp/src/qpid/types/Uuid.cpp b/qpid/cpp/src/qpid/types/Uuid.cpp
index 875e5925a9..3d31ab45c9 100644
--- a/qpid/cpp/src/qpid/types/Uuid.cpp
+++ b/qpid/cpp/src/qpid/types/Uuid.cpp
@@ -71,14 +71,13 @@ void Uuid::generate()
 
 void Uuid::clear()
 {
-    uuid_clear(bytes);
+    ::memset(bytes, 0, Uuid::SIZE);
 }
 
-// Force int 0/!0 to false/true; avoids compile warnings.
 bool Uuid::isNull() const
 {
-    // This const cast is for Solaris which has non const arguments
-    return !!uuid_is_null(const_cast<uint8_t*>(bytes));
+    static Uuid nullUuid;
+    return *this == nullUuid;
 }
 
 Uuid::operator bool() const { return !isNull(); }
@@ -93,8 +92,7 @@ const unsigned char* Uuid::data() const
 
 bool operator==(const Uuid& a, const Uuid& b)
 {
-    // This const cast is for Solaris which has non const arguments
-    return uuid_compare(const_cast<uint8_t*>(a.bytes), const_cast<uint8_t*>(b.bytes)) == 0;
+    return ::memcmp(a.bytes, b.bytes, Uuid::SIZE) == 0;
 }
 
 bool operator!=(const Uuid& a, const Uuid& b)
@@ -104,26 +102,22 @@ bool operator!=(const Uuid& a, const Uuid& b)
 
 bool operator<(const Uuid& a, const Uuid& b)
 {
-    // This const cast is for Solaris which has non const arguments
-    return uuid_compare(const_cast<uint8_t*>(a.bytes), const_cast<uint8_t*>(b.bytes)) < 0;
+    return ::memcmp(a.bytes, b.bytes, Uuid::SIZE) < 0;
 }
 
 bool operator>(const Uuid& a, const Uuid& b)
 {
-    // This const cast is for Solaris which has non const arguments
-    return uuid_compare(const_cast<uint8_t*>(a.bytes), const_cast<uint8_t*>(b.bytes)) > 0;
+    return ::memcmp(a.bytes, b.bytes, Uuid::SIZE) > 0;
 }
 
 bool operator<=(const Uuid& a, const Uuid& b)
 {
-    // This const cast is for Solaris which has non const arguments
-    return uuid_compare(const_cast<uint8_t*>(a.bytes), const_cast<uint8_t*>(b.bytes)) <= 0;
+    return ::memcmp(a.bytes, b.bytes, Uuid::SIZE) <= 0;
 }
 
 bool operator>=(const Uuid& a, const Uuid& b)
 {
-    // This const cast is for Solaris which has non const arguments
-    return uuid_compare(const_cast<uint8_t*>(a.bytes), const_cast<uint8_t*>(b.bytes)) >= 0;
+    return ::memcmp(a.bytes, b.bytes, Uuid::SIZE) >= 0;
 }
 
 ostream& operator<<(ostream& out, Uuid uuid)
diff --git a/qpid/cpp/src/tests/CMakeLists.txt b/qpid/cpp/src/tests/CMakeLists.txt
index cc57f8f773..9cacd10976 100644
--- a/qpid/cpp/src/tests/CMakeLists.txt
+++ b/qpid/cpp/src/tests/CMakeLists.txt
@@ -85,7 +85,7 @@ target_link_libraries (qpid-client-test qpidclient qpidcommon "${Boost_PROGRAM_O
 remember_location(qpid-client-test)
 
 add_executable (qpid-ping qpid-ping.cpp ${platform_test_additions})
-target_link_libraries (qpid-ping qpidclient qpidcommon "${Boost_PROGRAM_OPTIONS_LIBRARY}")
+target_link_libraries (qpid-ping qpidclient qpidcommon qpidtypes "${Boost_PROGRAM_OPTIONS_LIBRARY}")
 remember_location(qpid-ping)
 
 add_executable (qpid-topic-listener qpid-topic-listener.cpp ${platform_test_additions})
@@ -106,7 +106,7 @@ target_link_libraries (sender qpidmessaging qpidtypes qpidclient qpidcommon "${B
 remember_location(sender)
 
 add_executable (qpid-txtest qpid-txtest.cpp ${platform_test_additions})
-target_link_libraries (qpid-txtest qpidclient qpidcommon "${Boost_PROGRAM_OPTIONS_LIBRARY}")
+target_link_libraries (qpid-txtest qpidclient qpidcommon qpidtypes "${Boost_PROGRAM_OPTIONS_LIBRARY}")
 #qpid_txtest_SOURCES=qpid-txtest.cpp  TestOptions.h ConnectionOptions.h
 remember_location(qpid-txtest)
 
diff --git a/qpid/cpp/src/tests/Uuid.cpp b/qpid/cpp/src/tests/Uuid.cpp
index aa9580e25e..5fb848a500 100644
--- a/qpid/cpp/src/tests/Uuid.cpp
+++ b/qpid/cpp/src/tests/Uuid.cpp
@@ -24,6 +24,8 @@
 
 #include <set>
 
+#include <boost/array.hpp>
+
 namespace qpid {
 namespace tests {
 
@@ -51,17 +53,12 @@ boost::array<uint8_t, 16>  sample =  {{0x1b, 0x4e, 0x28, 0xba, 0x2f, 0xa1, 0x11,
 const string sampleStr("1b4e28ba-2fa1-11d2-883f-b9a761bde3fb");
 const string zeroStr("00000000-0000-0000-0000-000000000000");
 
-QPID_AUTO_TEST_CASE(testUuidStr) {
-    Uuid uuid(sampleStr);
-    BOOST_CHECK(uuid == sample);
-}
-
 QPID_AUTO_TEST_CASE(testUuidIstream) {
     Uuid uuid;
     istringstream in(sampleStr);
     in >> uuid;
     BOOST_CHECK(!in.fail());
-    BOOST_CHECK(uuid == sample);
+    BOOST_CHECK(::memcmp(uuid.data(), sample.data(), uuid.size())==0);
 
     istringstream is(zeroStr);
     Uuid zero;
@@ -105,7 +102,7 @@ QPID_AUTO_TEST_CASE(testUuidEncodeDecode) {
     Uuid decoded;
     decoded.decode(rbuf);
     BOOST_CHECK_EQUAL(string(sample.begin(), sample.end()),
-                      string(decoded.begin(), decoded.end()));
+                      string(decoded.data(), decoded.data()+decoded.size()));
 }
 
 QPID_AUTO_TEST_CASE(testTypesUuid)