From 8d4e8f60c9b73e3d15aa88a51d27ffcfc6fc1c40 Mon Sep 17 00:00:00 2001 From: AlessioBugetti Date: Tue, 20 Aug 2024 15:24:35 +0200 Subject: [PATCH] config-store, core: (fixes #1091) Fix handling of deprecated and obsolete attributes See also discussion in merge request !2109 --- src/config-store/model/attribute-iterator.cc | 9 +- src/config-store/model/config-store.cc | 16 ++-- src/config-store/model/file-config.cc | 6 -- src/config-store/model/file-config.h | 8 -- src/config-store/model/raw-text-config.cc | 91 +++++++++++--------- src/config-store/model/xml-config.cc | 84 ++++++++++-------- src/core/model/object-base.cc | 4 +- src/core/model/object-base.h | 5 +- src/core/model/type-id.cc | 81 ++++++++++------- src/core/model/type-id.h | 18 +++- 10 files changed, 186 insertions(+), 136 deletions(-) diff --git a/src/config-store/model/attribute-iterator.cc b/src/config-store/model/attribute-iterator.cc index efa512b78..80d7d899c 100644 --- a/src/config-store/model/attribute-iterator.cc +++ b/src/config-store/model/attribute-iterator.cc @@ -220,8 +220,13 @@ AttributeIterator::DoIterate(Ptr object) if (ptrChecker != nullptr) { NS_LOG_DEBUG("pointer attribute " << info.name); + if (info.supportLevel == TypeId::DEPRECATED || + info.supportLevel == TypeId::OBSOLETE) + { + continue; + } PointerValue ptr; - object->GetAttribute(info.name, ptr); + object->GetAttribute(info.name, ptr, true); Ptr tmp = ptr.Get(); if (tmp) { @@ -240,7 +245,7 @@ AttributeIterator::DoIterate(Ptr object) { NS_LOG_DEBUG("ObjectPtrContainer attribute " << info.name); ObjectPtrContainerValue vector; - object->GetAttribute(info.name, vector); + object->GetAttribute(info.name, vector, true); StartVisitArrayAttribute(object, info.name, vector); ObjectPtrContainerValue::Iterator it; for (it = vector.Begin(); it != vector.End(); ++it) diff --git a/src/config-store/model/config-store.cc b/src/config-store/model/config-store.cc index 36e60cc3e..3b49bfb6f 100644 --- a/src/config-store/model/config-store.cc +++ b/src/config-store/model/config-store.cc @@ -63,11 +63,15 @@ ConfigStore::GetTypeId() EnumValue(ConfigStore::RAW_TEXT), MakeEnumAccessor(&ConfigStore::SetFileFormat), MakeEnumChecker(ConfigStore::RAW_TEXT, "RawText", ConfigStore::XML, "Xml")) - .AddAttribute("SaveDeprecated", - "Save DEPRECATED attributes", - BooleanValue(true), - MakeBooleanAccessor(&ConfigStore::SetSaveDeprecated), - MakeBooleanChecker()); + .AddAttribute( + "SaveDeprecated", + "Save DEPRECATED attributes", + BooleanValue(true), + MakeBooleanAccessor(&ConfigStore::SetSaveDeprecated), + MakeBooleanChecker(), + TypeId::OBSOLETE, + "OBSOLETE since ns-3.43 as it is no longer needed; deprecated attributes are saved " + "only if their value differs from their respective original initial value"); return tid; } @@ -129,8 +133,6 @@ ConfigStore::ConfigStore() } } m_file->SetFilename(m_filename); - m_file->SetSaveDeprecated(m_saveDeprecated); - NS_LOG_FUNCTION(this << ": format: " << m_fileFormat << ", mode: " << m_mode << ", file name: " << m_filename); } diff --git a/src/config-store/model/file-config.cc b/src/config-store/model/file-config.cc index 6e390f0ac..79a24a3f6 100644 --- a/src/config-store/model/file-config.cc +++ b/src/config-store/model/file-config.cc @@ -15,12 +15,6 @@ FileConfig::~FileConfig() { } -void -FileConfig::SetSaveDeprecated(bool saveDeprecated) -{ - m_saveDeprecated = saveDeprecated; -} - NoneFileConfig::NoneFileConfig() { } diff --git a/src/config-store/model/file-config.h b/src/config-store/model/file-config.h index daacc9e55..61dd86b1e 100644 --- a/src/config-store/model/file-config.h +++ b/src/config-store/model/file-config.h @@ -28,11 +28,6 @@ class FileConfig * \param filename the filename */ virtual void SetFilename(std::string filename) = 0; - /** - * Set if to save deprecated attributes - * \param saveDeprecated the deprecated attributes save policy - */ - void SetSaveDeprecated(bool saveDeprecated); /** * Load or save the default values */ @@ -45,9 +40,6 @@ class FileConfig * Load or save the attributes values */ virtual void Attributes() = 0; - - protected: - bool m_saveDeprecated; ///< save deprecated attributes }; /** diff --git a/src/config-store/model/raw-text-config.cc b/src/config-store/model/raw-text-config.cc index bd69a0751..c508ac8f5 100644 --- a/src/config-store/model/raw-text-config.cc +++ b/src/config-store/model/raw-text-config.cc @@ -64,11 +64,6 @@ RawTextConfigSave::Default() m_os = os; } - void SetSaveDeprecated(bool saveDeprecated) - { - m_saveDeprecated = saveDeprecated; - } - private: void StartVisitTypeId(std::string name) override { @@ -80,39 +75,49 @@ RawTextConfigSave::Default() NS_LOG_DEBUG("Saving " << m_typeId << "::" << name); TypeId tid = TypeId::LookupByName(m_typeId); ns3::TypeId::SupportLevel supportLevel = TypeId::SupportLevel::SUPPORTED; + std::string originalInitialValue; + std::string valueTypeName; for (std::size_t i = 0; i < tid.GetAttributeN(); i++) { TypeId::AttributeInformation tmp = tid.GetAttribute(i); if (tmp.name == name) { supportLevel = tmp.supportLevel; + originalInitialValue = tmp.originalInitialValue->SerializeToString(tmp.checker); + valueTypeName = tmp.checker->GetValueTypeName(); break; } } + if (valueTypeName == "ns3::CallbackValue") + { + NS_LOG_WARN("Global attribute " << m_typeId << "::" << name + << " was not saved because it is a CallbackValue"); + return; + } if (supportLevel == TypeId::SupportLevel::OBSOLETE) { NS_LOG_WARN("Global attribute " << m_typeId << "::" << name << " was not saved because it is OBSOLETE"); + return; } - else if (supportLevel == TypeId::SupportLevel::DEPRECATED && !m_saveDeprecated) + if (supportLevel == TypeId::SupportLevel::DEPRECATED && + defaultValue == originalInitialValue) { - NS_LOG_WARN("Global attribute " << m_typeId << "::" << name - << " was not saved because it is DEPRECATED"); - } - else - { - *m_os << "default " << m_typeId << "::" << name << " \"" << defaultValue << "\"" - << std::endl; + NS_LOG_WARN("Global attribute " + << m_typeId << "::" << name + << " was not saved because it is DEPRECATED and its value has not " + "changed from the original initial value"); + return; } + *m_os << "default " << m_typeId << "::" << name << " \"" << defaultValue << "\"" + << std::endl; } std::string m_typeId; std::ostream* m_os; - bool m_saveDeprecated; }; RawTextDefaultIterator iterator = RawTextDefaultIterator(m_os); - iterator.SetSaveDeprecated(m_saveDeprecated); iterator.Iterate(); } @@ -142,52 +147,52 @@ RawTextConfigSave::Attributes() { } - void SetSaveDeprecated(bool saveDeprecated) - { - m_saveDeprecated = saveDeprecated; - } - private: void DoVisitAttribute(Ptr object, std::string name) override { StringValue str; - - ns3::TypeId::SupportLevel supportLevel = TypeId::SupportLevel::SUPPORTED; TypeId tid = object->GetInstanceTypeId(); - for (std::size_t i = 0; i < tid.GetAttributeN(); i++) + auto [found, inTid, attr] = TypeId::FindAttribute(tid, name); + + if (found) { - TypeId::AttributeInformation tmp = tid.GetAttribute(i); - if (tmp.name == name) + if (attr.checker && attr.checker->GetValueTypeName() == "ns3::CallbackValue") { - supportLevel = tmp.supportLevel; - break; + NS_LOG_WARN("Attribute " << GetCurrentPath() + << " was not saved because it is a CallbackValue"); + return; + } + auto supportLevel = attr.supportLevel; + if (supportLevel == TypeId::SupportLevel::OBSOLETE) + { + NS_LOG_WARN("Attribute " << GetCurrentPath() + << " was not saved because it is OBSOLETE"); + return; + } + + std::string originalInitialValue = + attr.originalInitialValue->SerializeToString(attr.checker); + object->GetAttribute(name, str, true); + + if (supportLevel == TypeId::SupportLevel::DEPRECATED && + str.Get() == originalInitialValue) + { + NS_LOG_WARN("Attribute " + << GetCurrentPath() + << " was not saved because it is DEPRECATED and its value has not " + "changed from the original initial value"); + return; } - } - if (supportLevel == TypeId::SupportLevel::OBSOLETE) - { - NS_LOG_WARN("Attribute " << GetCurrentPath() - << " was not saved because it is OBSOLETE"); - } - else if (supportLevel == TypeId::SupportLevel::DEPRECATED && !m_saveDeprecated) - { - NS_LOG_WARN("Attribute " << GetCurrentPath() - << " was not saved because it is DEPRECATED"); - } - else - { - object->GetAttribute(name, str); NS_LOG_DEBUG("Saving " << GetCurrentPath()); *m_os << "value " << GetCurrentPath() << " \"" << str.Get() << "\"" << std::endl; } } std::ostream* m_os; - bool m_saveDeprecated; }; RawTextAttributeIterator iter = RawTextAttributeIterator(m_os); - iter.SetSaveDeprecated(m_saveDeprecated); iter.Iterate(); } diff --git a/src/config-store/model/xml-config.cc b/src/config-store/model/xml-config.cc index e9d799120..564f20e15 100644 --- a/src/config-store/model/xml-config.cc +++ b/src/config-store/model/xml-config.cc @@ -104,11 +104,6 @@ XmlConfigSave::Default() m_writer = writer; } - void SetSaveDeprecated(bool saveDeprecated) - { - m_saveDeprecated = saveDeprecated; - } - private: void StartVisitTypeId(std::string name) override { @@ -119,25 +114,38 @@ XmlConfigSave::Default() { TypeId tid = TypeId::LookupByName(m_typeid); ns3::TypeId::SupportLevel supportLevel = TypeId::SupportLevel::SUPPORTED; + std::string originalInitialValue; + std::string valueTypeName; for (std::size_t i = 0; i < tid.GetAttributeN(); i++) { TypeId::AttributeInformation tmp = tid.GetAttribute(i); if (tmp.name == name) { supportLevel = tmp.supportLevel; + originalInitialValue = tmp.originalInitialValue->SerializeToString(tmp.checker); + valueTypeName = tmp.checker->GetValueTypeName(); break; } } + if (valueTypeName == "ns3::CallbackValue") + { + NS_LOG_WARN("Global attribute " << m_typeid << "::" << name + << " was not saved because it is a CallbackValue"); + return; + } if (supportLevel == TypeId::SupportLevel::OBSOLETE) { NS_LOG_WARN("Global attribute " << m_typeid << "::" << name << " was not saved because it is OBSOLETE"); return; } - else if (supportLevel == TypeId::SupportLevel::DEPRECATED && !m_saveDeprecated) + if (supportLevel == TypeId::SupportLevel::DEPRECATED && + defaultValue == originalInitialValue) { - NS_LOG_WARN("Global attribute " << m_typeid << "::" << name - << " was not saved because it is DEPRECATED"); + NS_LOG_WARN("Global attribute " + << m_typeid << "::" << name + << " was not saved because it is DEPRECATED and its value has not " + "changed from the original initial value"); return; } @@ -169,11 +177,9 @@ XmlConfigSave::Default() xmlTextWriterPtr m_writer; std::string m_typeid; - bool m_saveDeprecated; }; XmlDefaultIterator iterator = XmlDefaultIterator(m_writer); - iterator.SetSaveDeprecated(m_saveDeprecated); iterator.Iterate(); } @@ -188,39 +194,45 @@ XmlConfigSave::Attributes() { } - void SetSaveDeprecated(bool saveDeprecated) - { - m_saveDeprecated = saveDeprecated; - } - private: void DoVisitAttribute(Ptr object, std::string name) override { + StringValue str; TypeId tid = object->GetInstanceTypeId(); - ns3::TypeId::SupportLevel supportLevel = TypeId::SupportLevel::SUPPORTED; - for (std::size_t i = 0; i < tid.GetAttributeN(); i++) + + auto [found, inTid, attr] = TypeId::FindAttribute(tid, name); + + if (found) { - TypeId::AttributeInformation tmp = tid.GetAttribute(i); - if (tmp.name == name) + if (attr.checker && attr.checker->GetValueTypeName() == "ns3::CallbackValue") { - supportLevel = tmp.supportLevel; - break; + NS_LOG_WARN("Attribute " << GetCurrentPath() + << " was not saved because it is a CallbackValue"); + return; + } + auto supportLevel = attr.supportLevel; + if (supportLevel == TypeId::SupportLevel::OBSOLETE) + { + NS_LOG_WARN("Attribute " << GetCurrentPath() + << " was not saved because it is OBSOLETE"); + return; + } + + std::string originalInitialValue = + attr.originalInitialValue->SerializeToString(attr.checker); + object->GetAttribute(name, str, true); + + if (supportLevel == TypeId::SupportLevel::DEPRECATED && + str.Get() == originalInitialValue) + { + NS_LOG_WARN("Attribute " + << GetCurrentPath() + << " was not saved because it is DEPRECATED and its value has not " + "changed from the original initial value"); + return; } } - if (supportLevel == TypeId::SupportLevel::OBSOLETE) - { - NS_LOG_WARN("Attribute " << GetCurrentPath() - << " was not saved because it is OBSOLETE"); - return; - } - else if (supportLevel == TypeId::SupportLevel::DEPRECATED && !m_saveDeprecated) - { - NS_LOG_WARN("Attribute " << GetCurrentPath() - << " was not saved because it is DEPRECATED"); - return; - } - StringValue str; - object->GetAttribute(name, str); + int rc; rc = xmlTextWriterStartElement(m_writer, BAD_CAST "value"); if (rc < 0) @@ -248,11 +260,9 @@ XmlConfigSave::Attributes() } xmlTextWriterPtr m_writer; - bool m_saveDeprecated; }; XmlTextAttributeIterator iter = XmlTextAttributeIterator(m_writer); - iter.SetSaveDeprecated(m_saveDeprecated); iter.Iterate(); } diff --git a/src/core/model/object-base.cc b/src/core/model/object-base.cc index 94d4fe5ec..577311b0b 100644 --- a/src/core/model/object-base.cc +++ b/src/core/model/object-base.cc @@ -237,12 +237,12 @@ ObjectBase::SetAttributeFailSafe(std::string name, const AttributeValue& value) } void -ObjectBase::GetAttribute(std::string name, AttributeValue& value) const +ObjectBase::GetAttribute(std::string name, AttributeValue& value, bool permissive) const { NS_LOG_FUNCTION(this << name << &value); TypeId::AttributeInformation info; TypeId tid = GetInstanceTypeId(); - if (!tid.LookupAttributeByName(name, &info)) + if (!tid.LookupAttributeByName(name, &info, permissive)) { NS_FATAL_ERROR( "Attribute name=" << name << " does not exist for this object: tid=" << tid.GetName()); diff --git a/src/core/model/object-base.h b/src/core/model/object-base.h index d1b89cc50..c54f87fb6 100644 --- a/src/core/model/object-base.h +++ b/src/core/model/object-base.h @@ -224,8 +224,11 @@ class ObjectBase * * \param [in] name The name of the attribute to read. * \param [out] value Where the result should be stored. + * \param [in] permissive If false (by default), will generate warnings and errors for + * deprecated and obsolete attributes, respectively. If set to true, warnings for deprecated + * attributes will be suppressed. */ - void GetAttribute(std::string name, AttributeValue& value) const; + void GetAttribute(std::string name, AttributeValue& value, bool permissive = false) const; /** * Get the value of an attribute without raising errors. * diff --git a/src/core/model/type-id.cc b/src/core/model/type-id.cc index 5c27b825c..e0aa28640 100644 --- a/src/core/model/type-id.cc +++ b/src/core/model/type-id.cc @@ -936,41 +936,64 @@ TypeId::GetRegistered(uint16_t i) return TypeId(IidManager::Get()->GetRegistered(i)); } -bool -TypeId::LookupAttributeByName(std::string name, TypeId::AttributeInformation* info) const +std::tuple +TypeId::FindAttribute(const TypeId& tid, const std::string& name) { - NS_LOG_FUNCTION(this << name << info); - TypeId tid; - TypeId nextTid = *this; - do + TypeId currentTid = tid; + TypeId parentTid; + while (true) { - tid = nextTid; - for (std::size_t i = 0; i < tid.GetAttributeN(); i++) + for (std::size_t i = 0; i < currentTid.GetAttributeN(); ++i) { - TypeId::AttributeInformation tmp = tid.GetAttribute(i); - if (tmp.name == name) + const AttributeInformation& attributeInfo = currentTid.GetAttribute(i); + if (attributeInfo.name == name) { - if (tmp.supportLevel == TypeId::SUPPORTED) - { - *info = tmp; - return true; - } - else if (tmp.supportLevel == TypeId::DEPRECATED) - { - std::cerr << "Attribute '" << name << "' is deprecated: " << tmp.supportMsg - << std::endl; - *info = tmp; - return true; - } - else if (tmp.supportLevel == TypeId::OBSOLETE) - { - NS_FATAL_ERROR("Attribute '" << name << "' is obsolete, with no fallback: " - << tmp.supportMsg); - } + return {true, currentTid, attributeInfo}; } } - nextTid = tid.GetParent(); - } while (nextTid != tid); + + parentTid = currentTid.GetParent(); + + if (parentTid == currentTid) + { + break; + } + + currentTid = parentTid; + } + return {false, TypeId(), AttributeInformation()}; +} + +bool +TypeId::LookupAttributeByName(std::string name, + TypeId::AttributeInformation* info, + bool permissive) const +{ + NS_LOG_FUNCTION(this << name << info); + auto [found, tid, attribute] = FindAttribute(*this, name); + if (found) + { + if (attribute.supportLevel == TypeId::SUPPORTED) + { + *info = attribute; + return true; + } + else if (attribute.supportLevel == TypeId::DEPRECATED) + { + if (!permissive) + { + std::cerr << "Attribute '" << name << "' is deprecated: " << attribute.supportMsg + << std::endl; + } + *info = attribute; + return true; + } + else if (attribute.supportLevel == TypeId::OBSOLETE) + { + NS_FATAL_ERROR("Attribute '" + << name << "' is obsolete, with no fallback: " << attribute.supportMsg); + } + } return false; } diff --git a/src/core/model/type-id.h b/src/core/model/type-id.h index 9d0207d2c..52f5bfa47 100644 --- a/src/core/model/type-id.h +++ b/src/core/model/type-id.h @@ -467,6 +467,17 @@ class TypeId */ TypeId HideFromDocumentation(); + /** + * Find an attribute by name in the inheritance tree for a given TypeId. + * + * \param [in] tid The TypeId to start the search from. + * \param [in] name The name of the attribute to search for. + * \return A tuple containing a boolean that indicates whether the attribute was found, the + * TypeId where the attribute was found, and the AttributeInformation of the found attribute. + */ + static std::tuple FindAttribute(const TypeId& tid, + const std::string& name); + /** * Find an Attribute by name, retrieving the associated AttributeInformation. * @@ -474,9 +485,14 @@ class TypeId * \param [in,out] info A pointer to the TypeId::AttributeInformation * data structure where the result value of this method * will be stored. + * \param [in] permissive If false (by default), will generate warnings and errors for + * deprecated and obsolete attributes, respectively. If set to true, warnings for deprecated + * attributes will be suppressed. * \returns \c true if the requested attribute could be found. */ - bool LookupAttributeByName(std::string name, AttributeInformation* info) const; + bool LookupAttributeByName(std::string name, + AttributeInformation* info, + bool permissive = false) const; /** * Find a TraceSource by name. *