From 620d7a4181dda0bbf7c927a3dbfcf3973301369e Mon Sep 17 00:00:00 2001 From: Kamil Cudnik Date: Sat, 5 Dec 2020 12:30:19 +0100 Subject: [PATCH] Code clean (#721) --- lib/src/sai_redis_switch.cpp | 32 ++++--------------------------- meta/Meta.cpp | 15 --------------- meta/saiserialize.cpp | 14 +++++++------- syncd/AsicView.cpp | 11 +++++------ syncd/SaiDiscovery.cpp | 3 +-- syncd/SaiSwitch.cpp | 10 ++++------ syncd/Syncd.cpp | 2 -- syncd/ZeroMQSelectableChannel.cpp | 3 +-- vslib/src/NetMsgRegistrar.cpp | 3 ++- vslib/src/sai_vs_port.cpp | 2 +- 10 files changed, 25 insertions(+), 70 deletions(-) diff --git a/lib/src/sai_redis_switch.cpp b/lib/src/sai_redis_switch.cpp index 35f74e626ca0..00617eecd7be 100644 --- a/lib/src/sai_redis_switch.cpp +++ b/lib/src/sai_redis_switch.cpp @@ -1,6 +1,6 @@ #include "sai_redis.h" -sai_status_t redis_switch_mdio_read( +static sai_status_t redis_switch_mdio_read( _In_ sai_object_id_t switch_id, _In_ uint32_t device_addr, _In_ uint32_t start_reg_addr, @@ -12,7 +12,7 @@ sai_status_t redis_switch_mdio_read( return SAI_STATUS_NOT_IMPLEMENTED; } -sai_status_t redis_switch_mdio_write( +static sai_status_t redis_switch_mdio_write( _In_ sai_object_id_t switch_id, _In_ uint32_t device_addr, _In_ uint32_t start_reg_addr, @@ -41,30 +41,6 @@ static sai_status_t redis_create_switch_uniq( attr_list); } -static sai_status_t redis_mdio_read( - _In_ sai_object_id_t switch_id, - _In_ uint32_t device_addr, - _In_ uint32_t start_reg_addr, - _In_ uint32_t number_of_registers, - _Out_ uint32_t *reg_val) -{ - SWSS_LOG_ENTER(); - - return SAI_STATUS_NOT_IMPLEMENTED; -} - -static sai_status_t redis_mdio_write( - _In_ sai_object_id_t switch_id, - _In_ uint32_t device_addr, - _In_ uint32_t start_reg_addr, - _In_ uint32_t number_of_registers, - _In_ const uint32_t *reg_val) -{ - SWSS_LOG_ENTER(); - - return SAI_STATUS_NOT_IMPLEMENTED; -} - const sai_switch_api_t redis_switch_api = { redis_create_switch_uniq, @@ -74,6 +50,6 @@ const sai_switch_api_t redis_switch_api = { REDIS_GENERIC_STATS_API(switch) - redis_mdio_read, - redis_mdio_write, + redis_switch_mdio_read, + redis_switch_mdio_write, }; diff --git a/meta/Meta.cpp b/meta/Meta.cpp index 74a1d499dbdb..de3b0dbc0755 100644 --- a/meta/Meta.cpp +++ b/meta/Meta.cpp @@ -6083,21 +6083,6 @@ sai_status_t Meta::meta_generic_validate_non_object_on_create( sai_object_id_t oid = m->getoid(&meta_key); - if (oid == SAI_NULL_OBJECT_ID) - { - if (meta_key.objecttype == SAI_OBJECT_TYPE_FDB_ENTRY) - { - SWSS_LOG_WARN("workaround: %s is NULL, REMOVE when using bv_id", m->membername); - continue; - } - - SWSS_LOG_ERROR("oid on %s on struct member %s is NULL", - sai_serialize_object_type(meta_key.objecttype).c_str(), - m->membername); - - return SAI_STATUS_INVALID_PARAMETER; - } - if (oid == SAI_NULL_OBJECT_ID) { SWSS_LOG_ERROR("oid on %s on struct member %s is NULL", diff --git a/meta/saiserialize.cpp b/meta/saiserialize.cpp index 3a2f92439982..9c476f3cbc52 100644 --- a/meta/saiserialize.cpp +++ b/meta/saiserialize.cpp @@ -15,7 +15,7 @@ using json = nlohmann::json; -int char_to_int( +static int char_to_int( _In_ const char c) { SWSS_LOG_ENTER(); @@ -62,7 +62,7 @@ void sai_free_list( } template -void transfer_primitive( +static void transfer_primitive( _In_ const T &src_element, _In_ T &dst_element) { @@ -75,7 +75,7 @@ void transfer_primitive( } template -sai_status_t transfer_list( +static sai_status_t transfer_list( _In_ const T &src_element, _In_ T &dst_element, _In_ bool countOnly) @@ -457,7 +457,7 @@ sai_status_t transfer_attributes( // util -uint8_t get_ip_mask( +static uint8_t get_ip_mask( _In_ const uint8_t* mask, _In_ bool ipv6) { @@ -488,7 +488,7 @@ uint8_t get_ip_mask( return ones; } -uint8_t get_ipv4_mask( +static uint8_t get_ipv4_mask( _In_ const sai_ip4_t mask) { SWSS_LOG_ENTER(); @@ -496,7 +496,7 @@ uint8_t get_ipv4_mask( return get_ip_mask((const uint8_t*)&mask, false); } -int get_ipv6_mask( +static int get_ipv6_mask( _In_ const sai_ip6_t &mask) { SWSS_LOG_ENTER(); @@ -504,7 +504,7 @@ int get_ipv6_mask( return get_ip_mask(mask, true); } -void sai_populate_ip_mask( +static void sai_populate_ip_mask( _In_ uint8_t bits, _Out_ uint8_t *mask, _In_ bool ipv6) diff --git a/syncd/AsicView.cpp b/syncd/AsicView.cpp index c3be3463f55d..99965ef79c7c 100644 --- a/syncd/AsicView.cpp +++ b/syncd/AsicView.cpp @@ -829,7 +829,7 @@ void AsicView::asicRemoveObject( break; case SAI_OBJECT_TYPE_NAT_ENTRY: - m_soNatEntries[currentObj->m_str_object_id] = currentObj; + m_soNatEntries.erase(currentObj->m_str_object_id); break; default: @@ -1105,7 +1105,7 @@ void AsicView::dumpRef(const std::string & asicName) SWSS_LOG_NOTICE("dump references in ASIC VIEW: %s", asicName.c_str()); - for (auto kvp: m_vidReference) + for (auto& kvp: m_vidReference) { sai_object_id_t oid = kvp.first; @@ -1137,7 +1137,7 @@ void AsicView::dumpVidToAsicOperatioId() const { SWSS_LOG_ENTER(); - for (auto a: m_vidToAsicOperationId) + for (auto& a: m_vidToAsicOperationId) { auto ot = VidManager::objectTypeQuery(a.first); @@ -1153,7 +1153,7 @@ void AsicView::populateAttributes( { SWSS_LOG_ENTER(); - for (const auto &field: map) + for (const auto& field: map) { std::shared_ptr attr = std::make_shared(field.first, field.second); @@ -1163,7 +1163,6 @@ void AsicView::populateAttributes( switch (meta->attrid) { - case SAI_ACL_COUNTER_ATTR_PACKETS: case SAI_ACL_COUNTER_ATTR_BYTES: @@ -1178,13 +1177,13 @@ void AsicView::populateAttributes( break; } } + if (obj->getObjectType() == SAI_OBJECT_TYPE_NAT_ENTRY) { auto* meta = attr->getAttrMetadata(); switch (meta->attrid) { - case SAI_NAT_ENTRY_ATTR_HIT_BIT_COR: case SAI_NAT_ENTRY_ATTR_HIT_BIT: diff --git a/syncd/SaiDiscovery.cpp b/syncd/SaiDiscovery.cpp index 309871a50208..a241fabdbdae 100644 --- a/syncd/SaiDiscovery.cpp +++ b/syncd/SaiDiscovery.cpp @@ -18,7 +18,6 @@ using namespace syncd; */ #define SAI_DISCOVERY_LIST_MAX_ELEMENTS 1024 - SaiDiscovery::SaiDiscovery( _In_ std::shared_ptr sai): m_sai(sai) @@ -88,7 +87,7 @@ void SaiDiscovery::discover( discovered.insert(rid); } - const sai_object_type_info_t *info = sai_metadata_get_object_type_info(ot); + const sai_object_type_info_t *info = sai_metadata_get_object_type_info(ot); /* * We will query only oid object types diff --git a/syncd/SaiSwitch.cpp b/syncd/SaiSwitch.cpp index 6fdb902dad91..60eb01c237c0 100644 --- a/syncd/SaiSwitch.cpp +++ b/syncd/SaiSwitch.cpp @@ -79,7 +79,6 @@ SaiSwitch::SaiSwitch( } } - /* * NOTE: all those methods could be implemented inside SaiSwitch class so then * we could skip using switch_id in params and even they could be public then. @@ -749,7 +748,7 @@ std::set SaiSwitch::getWarmBootDiscoveredVids() const } void SaiSwitch::redisSaveInternalOids( - _In_ sai_object_id_t rid) const + _In_ sai_object_id_t rid) const { SWSS_LOG_ENTER(); @@ -767,13 +766,12 @@ void SaiSwitch::redisSaveInternalOids( * is SAI_SWITCH_ATTR_DEFAULT_STP_INST_ID which is discovered in warm-boot * when upgrading to new SAI Version*/ - m_client->setDummyAsicStateObject(vid); m_client->saveColdBootDiscoveredVids(m_switch_vid, coldVids); SWSS_LOG_NOTICE("put switch internal discovered rid %s to Asic View and COLDVIDS", - sai_serialize_object_id(rid).c_str()); + sai_serialize_object_id(rid).c_str()); } @@ -1144,7 +1142,7 @@ void SaiSwitch::postPortRemove( if (vid == SAI_NULL_OBJECT_ID) { SWSS_LOG_THROW("expected rid %s to be present in RIDTOVID", - sai_serialize_object_id(rid).c_str()); + sai_serialize_object_id(rid).c_str()); } // TODO should this remove rid,vid and object be as db op? @@ -1165,7 +1163,7 @@ void SaiSwitch::postPortRemove( if (removed == 0) { SWSS_LOG_THROW("NO LANES found in redis lane map for given port RID %s", - sai_serialize_object_id(portRid).c_str()); + sai_serialize_object_id(portRid).c_str()); } SWSS_LOG_NOTICE("post port remove actions succeeded"); diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index 44d5e68227c8..0e197baffef7 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -1925,8 +1925,6 @@ void Syncd::syncUpdateRedisQuadEvent( auto& values = kfvFieldsValues(kco); - const std::string& strObjectId = key.substr(key.find(":") + 1); - sai_object_meta_key_t metaKey; sai_deserialize_object_meta_key(key, metaKey); diff --git a/syncd/ZeroMQSelectableChannel.cpp b/syncd/ZeroMQSelectableChannel.cpp index fc891455464a..c1e383dea0b6 100644 --- a/syncd/ZeroMQSelectableChannel.cpp +++ b/syncd/ZeroMQSelectableChannel.cpp @@ -156,8 +156,7 @@ void ZeroMQSelectableChannel::pop( if (m_queue.empty()) { - SWSS_LOG_ERROR("queue is empty, can't pop"); - throw std::runtime_error("queue is empty, can't pop"); + SWSS_LOG_THROW("queue is empty, can't pop"); } std::string msg = m_queue.front(); diff --git a/vslib/src/NetMsgRegistrar.cpp b/vslib/src/NetMsgRegistrar.cpp index 186e9b7da208..c7f9798c8c2e 100644 --- a/vslib/src/NetMsgRegistrar.cpp +++ b/vslib/src/NetMsgRegistrar.cpp @@ -134,7 +134,8 @@ void NetMsgRegistrar::run() } } - // TODO unregister messages when swss common pointer will be advanced + swss::NetDispatcher::getInstance().unregisterMessageHandler(RTM_NEWLINK); + swss::NetDispatcher::getInstance().unregisterMessageHandler(RTM_DELLINK); SWSS_LOG_NOTICE("netlink msg listener ended"); } diff --git a/vslib/src/sai_vs_port.cpp b/vslib/src/sai_vs_port.cpp index 5eea79de476a..79bccf7771e1 100644 --- a/vslib/src/sai_vs_port.cpp +++ b/vslib/src/sai_vs_port.cpp @@ -1,6 +1,6 @@ #include "sai_vs.h" -sai_status_t vs_clear_port_all_stats( +static sai_status_t vs_clear_port_all_stats( _In_ sai_object_id_t port_id) { SWSS_LOG_ENTER();