From d6390a28188171dee745e12cca99af0f61468e2d Mon Sep 17 00:00:00 2001 From: Judson Wilson Date: Thu, 2 Jul 2026 02:59:14 +0300 Subject: [PATCH 1/4] Fix saisdkdump profile file flag. It was not parsing the kv pairs out of the file. This code is mostly copied from saidiscovery, and put into a separate file in a class for interfacing with unit tests. Signed-off-by: Judson Wilson --- .gitignore | 1 + configure.ac | 1 + saisdkdump/Makefile.am | 10 +- saisdkdump/ProfileMap.cpp | 130 +++++++++++++++++++++ saisdkdump/ProfileMap.h | 22 ++++ saisdkdump/saisdkdump.cpp | 15 ++- unittest/Makefile.am | 2 +- unittest/saisdkdump/Makefile.am | 13 +++ unittest/saisdkdump/TestProfileMap.cpp | 150 +++++++++++++++++++++++++ unittest/saisdkdump/main.cpp | 7 ++ 10 files changed, 345 insertions(+), 6 deletions(-) create mode 100644 saisdkdump/ProfileMap.cpp create mode 100644 saisdkdump/ProfileMap.h create mode 100644 unittest/saisdkdump/Makefile.am create mode 100644 unittest/saisdkdump/TestProfileMap.cpp create mode 100644 unittest/saisdkdump/main.cpp diff --git a/.gitignore b/.gitignore index db4a306321..140dda9f1e 100644 --- a/.gitignore +++ b/.gitignore @@ -119,6 +119,7 @@ unittest/vslib/testslibsaivs unittest/proxylib/tests unittest/proxylib/testslibsaiproxy unittest/saidump/tests +unittest/saisdkdump/tests vslib/tests stub/tests unittest/stub/stub/testslibsaistub diff --git a/configure.ac b/configure.ac index 9f862d56d5..feab9a743b 100644 --- a/configure.ac +++ b/configure.ac @@ -306,6 +306,7 @@ AC_OUTPUT(Makefile unittest/syncd/Makefile unittest/proxylib/Makefile unittest/saidump/Makefile + unittest/saisdkdump/Makefile pyext/Makefile pyext/py2/Makefile pyext/py3/Makefile) diff --git a/saisdkdump/Makefile.am b/saisdkdump/Makefile.am index 6c999d7f4c..54f5f6329d 100644 --- a/saisdkdump/Makefile.am +++ b/saisdkdump/Makefile.am @@ -1,4 +1,4 @@ -AM_CXXFLAGS = $(SAIINC) +AM_CXXFLAGS = $(SAIINC) -I$(top_srcdir)/saisdkdump bin_PROGRAMS = saisdkdump @@ -8,7 +8,13 @@ else SAILIB=-lsai endif +noinst_LIBRARIES = libsaisdkdump_profile.a + +libsaisdkdump_profile_a_SOURCES = ProfileMap.cpp +libsaisdkdump_profile_a_CPPFLAGS = $(CODE_COVERAGE_CPPFLAGS) +libsaisdkdump_profile_a_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) $(CODE_COVERAGE_CXXFLAGS) + saisdkdump_SOURCES = saisdkdump.cpp saisdkdump_CPPFLAGS = $(CODE_COVERAGE_CPPFLAGS) saisdkdump_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) $(CODE_COVERAGE_CXXFLAGS) -saisdkdump_LDADD = -lhiredis -lswsscommon $(SAILIB) -lpthread -L$(top_srcdir)/meta/.libs -lsaimetadata -lsaimeta -lzmq $(CODE_COVERAGE_LIBS) $(EXTRA_LIBSAI_LDFLAGS) +saisdkdump_LDADD = libsaisdkdump_profile.a -lhiredis -lswsscommon $(SAILIB) -lpthread -L$(top_srcdir)/meta/.libs -lsaimetadata -lsaimeta -lzmq $(CODE_COVERAGE_LIBS) $(EXTRA_LIBSAI_LDFLAGS) diff --git a/saisdkdump/ProfileMap.cpp b/saisdkdump/ProfileMap.cpp new file mode 100644 index 0000000000..8c0255bbcd --- /dev/null +++ b/saisdkdump/ProfileMap.cpp @@ -0,0 +1,130 @@ +#include "ProfileMap.h" + +#include +#include +#include + +#include "swss/logger.h" + +ProfileMap::ProfileMap() + : m_iter(m_map.begin()) +{ +} + +bool ProfileMap::loadFromFile(const std::string& profileMapFile) +{ + SWSS_LOG_ENTER(); + + m_map.clear(); + m_iter = m_map.begin(); + + if (profileMapFile.empty()) + { + return true; + } + + std::ifstream profile(profileMapFile); + + if (!profile.is_open()) + { + SWSS_LOG_ERROR("failed to open profile map file: '%s' : %s", + profileMapFile.c_str(), strerror(errno)); + + return false; + } + + std::string line; + + while (getline(profile, line)) + { + if (!line.empty() && line.back() == '\r') + { + line.pop_back(); + } + + if (line.size() > 0 && (line[0] == '#' || line[0] == ';')) + { + continue; + } + + size_t pos = line.find("="); + + if (pos == std::string::npos) + { + SWSS_LOG_WARN("not found '=' in line %s", line.c_str()); + continue; + } + + std::string key = line.substr(0, pos); + std::string value = line.substr(pos + 1); + + m_map[key] = value; + + SWSS_LOG_INFO("inserted: %s:%s", key.c_str(), value.c_str()); + } + + return true; +} + +void ProfileMap::clear() +{ + m_map.clear(); + m_iter = m_map.begin(); +} + +const char* ProfileMap::getValue(const char* variable) const +{ + SWSS_LOG_ENTER(); + + if (variable == NULL) + { + SWSS_LOG_WARN("variable is null"); + return NULL; + } + + auto it = m_map.find(variable); + + if (it == m_map.end()) + { + SWSS_LOG_NOTICE("%s: NULL", variable); + return NULL; + } + + SWSS_LOG_NOTICE("%s: %s", variable, it->second.c_str()); + + return it->second.c_str(); +} + +int ProfileMap::getNextValue(const char** variable, const char** value) +{ + SWSS_LOG_ENTER(); + + if (value == NULL) + { + SWSS_LOG_INFO("resetting profile map iterator"); + + m_iter = m_map.begin(); + return 0; + } + + if (variable == NULL) + { + SWSS_LOG_WARN("variable is null"); + return -1; + } + + if (m_iter == m_map.end()) + { + SWSS_LOG_INFO("iterator reached end"); + return -1; + } + + *variable = m_iter->first.c_str(); + *value = m_iter->second.c_str(); + + SWSS_LOG_INFO("key: %s:%s", *variable, *value); + + m_iter++; + + return 0; +} diff --git a/saisdkdump/ProfileMap.h b/saisdkdump/ProfileMap.h new file mode 100644 index 0000000000..6f67014dd1 --- /dev/null +++ b/saisdkdump/ProfileMap.h @@ -0,0 +1,22 @@ +#pragma once + +#include +#include + +class ProfileMap +{ +public: + ProfileMap(); + + bool loadFromFile(const std::string& profileMapFile); + + void clear(); + + const char* getValue(const char* variable) const; + + int getNextValue(const char** variable, const char** value); + +private: + std::map m_map; + mutable std::map::iterator m_iter; +}; diff --git a/saisdkdump/saisdkdump.cpp b/saisdkdump/saisdkdump.cpp index 57333988aa..14ebfac2ac 100644 --- a/saisdkdump/saisdkdump.cpp +++ b/saisdkdump/saisdkdump.cpp @@ -1,11 +1,12 @@ #include #include #include -#include #include #include +#include +#include "ProfileMap.h" #include "swss/logger.h" extern "C" { @@ -16,6 +17,8 @@ extern "C" { std::string sai_profile = "/tmp/sai.profile"; +static ProfileMap g_profileMap; + void print_usage() { SWSS_LOG_ENTER(); @@ -46,7 +49,7 @@ const char* profile_get_value( { SWSS_LOG_ENTER(); - return sai_profile.c_str(); + return g_profileMap.getValue(variable); } int profile_get_next_value( @@ -55,7 +58,8 @@ int profile_get_next_value( _Out_ const char** value) { SWSS_LOG_ENTER(); - return -1; + + return g_profileMap.getNextValue(variable, value); } sai_service_method_table_t test_services = { @@ -120,6 +124,11 @@ int main(int argc, char **argv) SWSS_LOG_INFO("The dump file is not specified, generated \"%s\" file name", fileName.c_str()); } + if (!g_profileMap.loadFromFile(sai_profile)) + { + exit(EXIT_FAILURE); + } + sai_status_t status = sai_api_initialize(0, (sai_service_method_table_t*)&test_services); if (status != SAI_STATUS_SUCCESS) { diff --git a/unittest/Makefile.am b/unittest/Makefile.am index 6dbf301d82..effc05df7f 100644 --- a/unittest/Makefile.am +++ b/unittest/Makefile.am @@ -1 +1 @@ -SUBDIRS = meta lib vslib syncd proxylib saidump +SUBDIRS = meta lib vslib syncd proxylib saidump saisdkdump diff --git a/unittest/saisdkdump/Makefile.am b/unittest/saisdkdump/Makefile.am new file mode 100644 index 0000000000..1dea67fbee --- /dev/null +++ b/unittest/saisdkdump/Makefile.am @@ -0,0 +1,13 @@ +AM_CXXFLAGS = $(SAIINC) -I$(top_srcdir)/saisdkdump + +bin_PROGRAMS = tests + +LDADD_GTEST = -L/usr/src/gtest -lgtest -lgtest_main + +tests_SOURCES = main.cpp TestProfileMap.cpp + +tests_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) $(CODE_COVERAGE_CXXFLAGS) +tests_LDADD = $(LDADD_GTEST) $(top_srcdir)/saisdkdump/libsaisdkdump_profile.a \ + -lswsscommon -lpthread $(CODE_COVERAGE_LIBS) + +TESTS = tests diff --git a/unittest/saisdkdump/TestProfileMap.cpp b/unittest/saisdkdump/TestProfileMap.cpp new file mode 100644 index 0000000000..2a2ab3d6ea --- /dev/null +++ b/unittest/saisdkdump/TestProfileMap.cpp @@ -0,0 +1,150 @@ +#include + +#include +#include +#include +#include +#include + +#include "ProfileMap.h" + +namespace +{ + class TempProfile + { + public: + explicit TempProfile(const std::string& content) + { + char path[] = "/tmp/saisdkdump_profile_test.XXXXXX"; + int fd = mkstemp(path); + EXPECT_GE(fd, 0); + if (fd < 0) + { + return; + } + + m_path = path; + close(fd); + + std::ofstream out(m_path); + out << content; + } + + ~TempProfile() + { + if (!m_path.empty()) + { + unlink(m_path.c_str()); + } + } + + const std::string& path() const + { + return m_path; + } + + TempProfile(const TempProfile&) = delete; + TempProfile& operator=(const TempProfile&) = delete; + + private: + std::string m_path; + }; +} + +TEST(ProfileMap, emptyPathLoadsNoEntries) +{ + ProfileMap profileMap; + + EXPECT_TRUE(profileMap.loadFromFile("")); + EXPECT_EQ(nullptr, profileMap.getValue("CT_TABLE_DUMP_ENABLE")); +} + +TEST(ProfileMap, missingFileReturnsFalse) +{ + ProfileMap profileMap; + + EXPECT_FALSE(profileMap.loadFromFile("/tmp/saisdkdump_profile_missing_file")); +} + +TEST(ProfileMap, parsesKeyValuePairs) +{ + ProfileMap profileMap; + const TempProfile profile( + "# comment line\n" + "; comment line\n" + "CT_TABLE_DUMP_ENABLE=true\n" + "OTHER_KEY=value\n"); + + EXPECT_TRUE(profileMap.loadFromFile(profile.path())); + EXPECT_STREQ("true", profileMap.getValue("CT_TABLE_DUMP_ENABLE")); + EXPECT_STREQ("value", profileMap.getValue("OTHER_KEY")); + EXPECT_EQ(nullptr, profileMap.getValue("UNKNOWN_KEY")); +} + +TEST(ProfileMap, parsesCrLfLineEndings) +{ + ProfileMap profileMap; + const TempProfile profile( + "CT_TABLE_DUMP_ENABLE=true\r\n" + "OTHER_KEY=value\r\n"); + + EXPECT_TRUE(profileMap.loadFromFile(profile.path())); + EXPECT_STREQ("true", profileMap.getValue("CT_TABLE_DUMP_ENABLE")); + EXPECT_STREQ("value", profileMap.getValue("OTHER_KEY")); +} + +TEST(ProfileMap, skipsMalformedLines) +{ + ProfileMap profileMap; + const TempProfile profile( + "VALID=yes\n" + "no_equals_sign\n" + "CT_TABLE_DUMP_ENABLE=true\n"); + + EXPECT_TRUE(profileMap.loadFromFile(profile.path())); + EXPECT_STREQ("yes", profileMap.getValue("VALID")); + EXPECT_STREQ("true", profileMap.getValue("CT_TABLE_DUMP_ENABLE")); +} + +TEST(ProfileMap, getNextValueIteratesAndResets) +{ + ProfileMap profileMap; + const TempProfile profile( + "FIRST=one\n" + "SECOND=two\n"); + + EXPECT_TRUE(profileMap.loadFromFile(profile.path())); + + const char* variable = nullptr; + const char* value = nullptr; + + EXPECT_EQ(0, profileMap.getNextValue(nullptr, nullptr)); + + std::vector> entries; + while (profileMap.getNextValue(&variable, &value) == 0) + { + entries.emplace_back(variable, value); + } + + ASSERT_EQ(2u, entries.size()); + EXPECT_EQ("FIRST", entries[0].first); + EXPECT_EQ("one", entries[0].second); + EXPECT_EQ("SECOND", entries[1].first); + EXPECT_EQ("two", entries[1].second); + + EXPECT_EQ(0, profileMap.getNextValue(nullptr, nullptr)); + EXPECT_EQ(0, profileMap.getNextValue(&variable, &value)); + EXPECT_EQ("FIRST", std::string(variable)); +} + +TEST(ProfileMap, clearRemovesEntries) +{ + ProfileMap profileMap; + const TempProfile profile("CT_TABLE_DUMP_ENABLE=true\n"); + + EXPECT_TRUE(profileMap.loadFromFile(profile.path())); + EXPECT_STREQ("true", profileMap.getValue("CT_TABLE_DUMP_ENABLE")); + + profileMap.clear(); + EXPECT_EQ(nullptr, profileMap.getValue("CT_TABLE_DUMP_ENABLE")); +} diff --git a/unittest/saisdkdump/main.cpp b/unittest/saisdkdump/main.cpp new file mode 100644 index 0000000000..d973578fc9 --- /dev/null +++ b/unittest/saisdkdump/main.cpp @@ -0,0 +1,7 @@ +#include + +int main(int argc, char *argv[]) +{ + testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} From 3b90ef35a898e06d0035cfceda65882b8de26343 Mon Sep 17 00:00:00 2001 From: Judson Wilson Date: Thu, 16 Jul 2026 01:18:35 +0300 Subject: [PATCH 2/4] Fix missing SWSS_LOG_ENTER() Signed-off-by: Judson Wilson --- saisdkdump/ProfileMap.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/saisdkdump/ProfileMap.cpp b/saisdkdump/ProfileMap.cpp index 8c0255bbcd..ce8a12d36e 100644 --- a/saisdkdump/ProfileMap.cpp +++ b/saisdkdump/ProfileMap.cpp @@ -68,6 +68,8 @@ bool ProfileMap::loadFromFile(const std::string& profileMapFile) void ProfileMap::clear() { + SWSS_LOG_ENTER(); + m_map.clear(); m_iter = m_map.begin(); } From 765b6543e73abfc86c5fb33d045819c05bc07809 Mon Sep 17 00:00:00 2001 From: Judson Wilson Date: Thu, 16 Jul 2026 01:55:31 +0300 Subject: [PATCH 3/4] Add SWSS_LOG_ENTER() to unit tests. Signed-off-by: Judson Wilson --- unittest/saisdkdump/TestProfileMap.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/unittest/saisdkdump/TestProfileMap.cpp b/unittest/saisdkdump/TestProfileMap.cpp index 2a2ab3d6ea..798df0ecd8 100644 --- a/unittest/saisdkdump/TestProfileMap.cpp +++ b/unittest/saisdkdump/TestProfileMap.cpp @@ -15,6 +15,8 @@ namespace public: explicit TempProfile(const std::string& content) { + SWSS_LOG_ENTER(); + char path[] = "/tmp/saisdkdump_profile_test.XXXXXX"; int fd = mkstemp(path); EXPECT_GE(fd, 0); @@ -32,6 +34,8 @@ namespace ~TempProfile() { + SWSS_LOG_ENTER(); + if (!m_path.empty()) { unlink(m_path.c_str()); @@ -40,6 +44,8 @@ namespace const std::string& path() const { + SWSS_LOG_ENTER(); + return m_path; } @@ -53,6 +59,8 @@ namespace TEST(ProfileMap, emptyPathLoadsNoEntries) { + SWSS_LOG_ENTER(); + ProfileMap profileMap; EXPECT_TRUE(profileMap.loadFromFile("")); @@ -61,6 +69,8 @@ TEST(ProfileMap, emptyPathLoadsNoEntries) TEST(ProfileMap, missingFileReturnsFalse) { + SWSS_LOG_ENTER(); + ProfileMap profileMap; EXPECT_FALSE(profileMap.loadFromFile("/tmp/saisdkdump_profile_missing_file")); @@ -68,6 +78,8 @@ TEST(ProfileMap, missingFileReturnsFalse) TEST(ProfileMap, parsesKeyValuePairs) { + SWSS_LOG_ENTER(); + ProfileMap profileMap; const TempProfile profile( "# comment line\n" @@ -83,6 +95,8 @@ TEST(ProfileMap, parsesKeyValuePairs) TEST(ProfileMap, parsesCrLfLineEndings) { + SWSS_LOG_ENTER(); + ProfileMap profileMap; const TempProfile profile( "CT_TABLE_DUMP_ENABLE=true\r\n" @@ -95,6 +109,8 @@ TEST(ProfileMap, parsesCrLfLineEndings) TEST(ProfileMap, skipsMalformedLines) { + SWSS_LOG_ENTER(); + ProfileMap profileMap; const TempProfile profile( "VALID=yes\n" @@ -108,6 +124,8 @@ TEST(ProfileMap, skipsMalformedLines) TEST(ProfileMap, getNextValueIteratesAndResets) { + SWSS_LOG_ENTER(); + ProfileMap profileMap; const TempProfile profile( "FIRST=one\n" @@ -139,6 +157,8 @@ TEST(ProfileMap, getNextValueIteratesAndResets) TEST(ProfileMap, clearRemovesEntries) { + SWSS_LOG_ENTER(); + ProfileMap profileMap; const TempProfile profile("CT_TABLE_DUMP_ENABLE=true\n"); From 7363cb5ad6e34900349b2e6b7d7767bde49decc7 Mon Sep 17 00:00:00 2001 From: Judson Wilson Date: Thu, 16 Jul 2026 02:23:20 +0300 Subject: [PATCH 4/4] Add missing include for swss/logger.h Signed-off-by: Judson Wilson --- unittest/saisdkdump/TestProfileMap.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/unittest/saisdkdump/TestProfileMap.cpp b/unittest/saisdkdump/TestProfileMap.cpp index 798df0ecd8..9875d596b1 100644 --- a/unittest/saisdkdump/TestProfileMap.cpp +++ b/unittest/saisdkdump/TestProfileMap.cpp @@ -8,6 +8,8 @@ #include "ProfileMap.h" +#include "swss/logger.h" + namespace { class TempProfile