diff --git a/libuavcan/include/uavcan/data_type.hpp b/libuavcan/include/uavcan/data_type.hpp index 345bbafa7a..754106e500 100644 --- a/libuavcan/include/uavcan/data_type.hpp +++ b/libuavcan/include/uavcan/data_type.hpp @@ -118,6 +118,7 @@ public: bool match(DataTypeKind kind, const char* name) const; bool match(DataTypeKind kind, uint16_t id) const; + bool match(const char* name) const { return match(kind_, name); } std::string toString() const; diff --git a/libuavcan/include/uavcan/global_data_type_registry.hpp b/libuavcan/include/uavcan/global_data_type_registry.hpp index 7b0e4663a1..d1a45ea21e 100644 --- a/libuavcan/include/uavcan/global_data_type_registry.hpp +++ b/libuavcan/include/uavcan/global_data_type_registry.hpp @@ -41,6 +41,16 @@ class GlobalDataTypeRegistry : Noncopyable } }; +public: + enum RegistResult + { + RegistResultOk, + RegistResultCollision, + RegistResultInvalidParams, + RegistResultFrozen + }; + +private: typedef LinkedListRoot List; List msgs_; List srvs_; @@ -51,25 +61,29 @@ class GlobalDataTypeRegistry : Noncopyable const List* selectList(DataTypeKind kind) const; List* selectList(DataTypeKind kind); - void remove(Entry* dtd); - bool add(Entry* dtd); + RegistResult remove(Entry* dtd); + RegistResult registImpl(Entry* dtd); public: static GlobalDataTypeRegistry& instance(); /// Last call wins template - bool assign(uint16_t id) + RegistResult regist(uint16_t id) { if (isFrozen()) - return false; + return RegistResultFrozen; + static Entry entry; - remove(&entry); + const RegistResult remove_res = remove(&entry); + if (remove_res != RegistResultOk) + return remove_res; + entry = Entry(DataTypeKind(Type::DataTypeKind), id, Type::getDataTypeSignature(), Type::getDataTypeName()); - return add(&entry); + return registImpl(&entry); } - /// Further calls to add() will fail + /// Further calls to regist<>() will fail void freeze() { frozen_ = true; } bool isFrozen() const { return frozen_; } @@ -101,7 +115,10 @@ struct DefaultDataTypeRegistrator { DefaultDataTypeRegistrator() { - if (!GlobalDataTypeRegistry::instance().assign(Type::DefaultDataTypeID)) + const GlobalDataTypeRegistry::RegistResult res = + GlobalDataTypeRegistry::instance().regist(Type::DefaultDataTypeID); + + if (res != GlobalDataTypeRegistry::RegistResultOk) { #if UAVCAN_EXCEPTIONS throw std::logic_error("Type registration failed"); diff --git a/libuavcan/src/global_data_type_registry.cpp b/libuavcan/src/global_data_type_registry.cpp index ef2eec4b20..9920d781ec 100644 --- a/libuavcan/src/global_data_type_registry.cpp +++ b/libuavcan/src/global_data_type_registry.cpp @@ -33,15 +33,19 @@ GlobalDataTypeRegistry::List* GlobalDataTypeRegistry::selectList(DataTypeKind ki } } -void GlobalDataTypeRegistry::remove(Entry* dtd) +GlobalDataTypeRegistry::RegistResult GlobalDataTypeRegistry::remove(Entry* dtd) { - assert(dtd); + if (!dtd) + { + assert(0); + return RegistResultInvalidParams; + } if (isFrozen()) - return; + return RegistResultFrozen; List* list = selectList(dtd->descriptor.getKind()); if (!list) - return; + return RegistResultInvalidParams; Entry* p = list->get(); while (p) @@ -51,24 +55,31 @@ void GlobalDataTypeRegistry::remove(Entry* dtd) list->remove(dtd); p = next; } + return RegistResultOk; } -bool GlobalDataTypeRegistry::add(Entry* dtd) +GlobalDataTypeRegistry::RegistResult GlobalDataTypeRegistry::registImpl(Entry* dtd) { - assert(dtd); + if (!dtd || (dtd->descriptor.getID() > DataTypeDescriptor::MaxDataTypeID)) + { + assert(0); + return RegistResultInvalidParams; + } if (isFrozen()) - return false; + return RegistResultFrozen; List* list = selectList(dtd->descriptor.getKind()); if (!list) - return false; + return RegistResultInvalidParams; { // Collision check Entry* p = list->get(); while (p) { if (p->descriptor.getID() == dtd->descriptor.getID()) // ID collision - return false; + return RegistResultCollision; + if (p->descriptor.match(dtd->descriptor.getName())) // Name collision + return RegistResultCollision; p = p->getNextListNode(); } } @@ -90,7 +101,7 @@ bool GlobalDataTypeRegistry::add(Entry* dtd) } } #endif - return true; + return RegistResultOk; } GlobalDataTypeRegistry& GlobalDataTypeRegistry::instance() @@ -102,7 +113,7 @@ GlobalDataTypeRegistry& GlobalDataTypeRegistry::instance() const DataTypeDescriptor* GlobalDataTypeRegistry::find(DataTypeKind kind, const char* name) const { const List* list = selectList(kind); - if (list == NULL) + if (!list) { assert(0); return NULL; @@ -123,7 +134,7 @@ DataTypeSignature GlobalDataTypeRegistry::computeAggregateSignature(DataTypeKind assert(isFrozen()); // Computing the signature if the registry is not frozen is pointless const List* list = selectList(kind); - if (list == NULL) + if (!list) { assert(0); return DataTypeSignature(); @@ -174,7 +185,7 @@ void GlobalDataTypeRegistry::getDataTypeIDMask(DataTypeKind kind, DataTypeIDMask { mask.reset(); const List* list = selectList(kind); - if (list == NULL) + if (!list) { assert(0); return; diff --git a/libuavcan/test/global_data_type_registry.cpp b/libuavcan/test/global_data_type_registry.cpp index f8df40ccd1..dbe12809c0 100644 --- a/libuavcan/test/global_data_type_registry.cpp +++ b/libuavcan/test/global_data_type_registry.cpp @@ -47,6 +47,14 @@ namespace static const char* getDataTypeName() { return "foo.DataTypeD"; } }; + struct DataTypeBNameCollision + { + enum { DefaultDataTypeID = 999 }; + enum { DataTypeKind = uavcan::DataTypeKindMessage }; + static uavcan::DataTypeSignature getDataTypeSignature() { return uavcan::DataTypeSignature(963); } + static const char* getDataTypeName() { return "my_namespace.DataTypeB"; } + }; + template uavcan::DataTypeDescriptor extractDescriptor(uint16_t dtid = Type::DefaultDataTypeID) @@ -94,7 +102,8 @@ TEST(GlobalDataTypeRegistry, Basic) /* * Runtime registrations */ - ASSERT_TRUE(GlobalDataTypeRegistry::instance().assign(DataTypeAService::DefaultDataTypeID)); + ASSERT_EQ(GlobalDataTypeRegistry::RegistResultOk, + GlobalDataTypeRegistry::instance().regist(DataTypeAService::DefaultDataTypeID)); ASSERT_EQ(2, GlobalDataTypeRegistry::instance().getNumMessageTypes()); ASSERT_EQ(1, GlobalDataTypeRegistry::instance().getNumServiceTypes()); @@ -107,8 +116,8 @@ TEST(GlobalDataTypeRegistry, Basic) /* * Runtime re-registration */ - ASSERT_TRUE(GlobalDataTypeRegistry::instance().assign(147)); - ASSERT_TRUE(GlobalDataTypeRegistry::instance().assign(741)); + ASSERT_EQ(GlobalDataTypeRegistry::RegistResultOk, GlobalDataTypeRegistry::instance().regist(147)); + ASSERT_EQ(GlobalDataTypeRegistry::RegistResultOk, GlobalDataTypeRegistry::instance().regist(741)); ASSERT_EQ(2, GlobalDataTypeRegistry::instance().getNumMessageTypes()); ASSERT_EQ(1, GlobalDataTypeRegistry::instance().getNumServiceTypes()); @@ -127,8 +136,15 @@ TEST(GlobalDataTypeRegistry, Basic) /* * These types will be necessary for the aggregate signature test */ - ASSERT_FALSE(GlobalDataTypeRegistry::instance().assign(741)); // COLLISION - error - ASSERT_TRUE(GlobalDataTypeRegistry::instance().assign(DataTypeC::DefaultDataTypeID)); + ASSERT_EQ(GlobalDataTypeRegistry::RegistResultCollision, + GlobalDataTypeRegistry::instance().regist(741)); // ID COLLISION + + ASSERT_EQ(GlobalDataTypeRegistry::RegistResultCollision, + GlobalDataTypeRegistry::instance().regist( + DataTypeBNameCollision::DefaultDataTypeID)); // NAME COLLISION + + ASSERT_EQ(GlobalDataTypeRegistry::RegistResultOk, + GlobalDataTypeRegistry::instance().regist(DataTypeC::DefaultDataTypeID)); uavcan::DefaultDataTypeRegistrator reg_DataTypeD; GlobalDataTypeRegistry::instance().getDataTypeIDMask(uavcan::DataTypeKindMessage, dtmask); @@ -149,9 +165,14 @@ TEST(GlobalDataTypeRegistry, Basic) */ GlobalDataTypeRegistry::instance().freeze(); - ASSERT_FALSE(GlobalDataTypeRegistry::instance().assign(555)); // Rejected - ASSERT_FALSE(GlobalDataTypeRegistry::instance().assign(999)); // Rejected - ASSERT_FALSE(GlobalDataTypeRegistry::instance().assign(888)); // Rejected + ASSERT_EQ(GlobalDataTypeRegistry::RegistResultFrozen, + GlobalDataTypeRegistry::instance().regist(555)); // Rejected + + ASSERT_EQ(GlobalDataTypeRegistry::RegistResultFrozen, + GlobalDataTypeRegistry::instance().regist(999)); // Rejected + + ASSERT_EQ(GlobalDataTypeRegistry::RegistResultFrozen, + GlobalDataTypeRegistry::instance().regist(888)); // Rejected /* * Searching