add handler logic to handle SysCpldVersion
Add handler logic to handler for SysCpldVersion such that it splits the
true IPMI processing from the business logic.
Tested: Only ran unit-tests (added new ones).
Change-Id: I09d95d8be8fbe75648b3332af898336b00074c2f
Signed-off-by: Patrick Venture <venture@google.com>
diff --git a/cpld.cpp b/cpld.cpp
index 32c6f37..c3689cd 100644
--- a/cpld.cpp
+++ b/cpld.cpp
@@ -16,18 +16,16 @@
#include "cpld.hpp"
+#include "errors.hpp"
+#include "handler.hpp"
#include "main.hpp"
#include <cstring>
-#include <filesystem>
-#include <fstream>
-#include <sstream>
namespace google
{
namespace ipmi
{
-namespace fs = std::filesystem;
struct CpldRequest
{
@@ -48,7 +46,7 @@
// Handle reading the cpld version from the tmpfs.
//
ipmi_ret_t CpldVersion(const uint8_t* reqBuf, uint8_t* replyBuf,
- size_t* dataLen)
+ size_t* dataLen, const HandlerInterface* handler)
{
struct CpldRequest request;
@@ -68,59 +66,27 @@
// letter, because it does that.
std::memcpy(&request, &reqBuf[0], sizeof(request));
- std::ostringstream opath;
- opath << "/run/cpld" << static_cast<unsigned int>(request.id) << ".version";
- // Check for file
-
- std::error_code ec;
- if (!fs::exists(opath.str(), ec))
- {
- std::fprintf(stderr, "Path: '%s' doesn't exist.\n",
- opath.str().c_str());
- return IPMI_CC_INVALID_FIELD_REQUEST;
- }
- // We're uninterested in the state of ec.
-
- // If file exists, read.
- std::ifstream ifs;
- ifs.exceptions(std::ifstream::failbit);
- std::string value;
try
{
- ifs.open(opath.str());
- ifs >> value;
+ auto values =
+ handler->getCpldVersion(static_cast<unsigned int>(request.id));
+
+ // Truncate if the version is too high (documented).
+ struct CpldReply reply;
+ reply.subcommand = SysCpldVersion;
+ reply.major = std::get<0>(values);
+ reply.minor = std::get<1>(values);
+ reply.point = std::get<2>(values);
+ reply.subpoint = std::get<3>(values);
+
+ std::memcpy(&replyBuf[0], &reply, sizeof(reply));
+ (*dataLen) = sizeof(reply);
+ return IPMI_CC_OK;
}
- catch (std::ios_base::failure& fail)
+ catch (const IpmiException& e)
{
- return IPMI_CC_UNSPECIFIED_ERROR;
+ return e.getIpmiError();
}
-
- // If value parses as expected, return version.
- int major = 0;
- int minor = 0;
- int point = 0;
- int subpoint = 0;
-
- int num_fields =
- sscanf(value.c_str(), "%d.%d.%d.%d", &major, &minor, &point, &subpoint);
- if (num_fields == 0)
- {
- std::fprintf(stderr, "Invalid version.\n");
- return IPMI_CC_UNSPECIFIED_ERROR;
- }
-
- // Truncate if the version is too high (documented).
- struct CpldReply reply;
- reply.subcommand = SysCpldVersion;
- reply.major = static_cast<uint8_t>(major);
- reply.minor = static_cast<uint8_t>(minor);
- reply.point = static_cast<uint8_t>(point);
- reply.subpoint = static_cast<uint8_t>(subpoint);
-
- std::memcpy(&replyBuf[0], &reply, sizeof(reply));
- (*dataLen) = sizeof(reply);
-
- return IPMI_CC_OK;
}
} // namespace ipmi
diff --git a/cpld.hpp b/cpld.hpp
index 01550fe..bfb09ba 100644
--- a/cpld.hpp
+++ b/cpld.hpp
@@ -1,5 +1,7 @@
#pragma once
+#include "handler.hpp"
+
#include <ipmid/api.h>
namespace google
@@ -9,7 +11,8 @@
// Given a cpld identifier, return a version if available.
ipmi_ret_t CpldVersion(const uint8_t* reqBuf, uint8_t* replyBuf,
- size_t* dataLen);
+ size_t* dataLen,
+ const HandlerInterface* handler = &handlerImpl);
} // namespace ipmi
} // namespace google
diff --git a/handler.cpp b/handler.cpp
index 8d06f36..ac2ca3a 100644
--- a/handler.cpp
+++ b/handler.cpp
@@ -20,6 +20,7 @@
#include <ipmid/api.h>
+#include <cinttypes>
#include <cstdio>
#include <filesystem>
#include <fstream>
@@ -94,6 +95,51 @@
return count;
}
+VersionTuple Handler::getCpldVersion(unsigned int id) const
+{
+ std::ostringstream opath;
+ opath << "/run/cpld" << id << ".version";
+ // Check for file
+
+ std::error_code ec;
+ if (!fs::exists(opath.str(), ec))
+ {
+ std::fprintf(stderr, "Path: '%s' doesn't exist.\n",
+ opath.str().c_str());
+ throw IpmiException(IPMI_CC_INVALID_FIELD_REQUEST);
+ }
+ // We're uninterested in the state of ec.
+
+ // If file exists, read.
+ std::ifstream ifs;
+ ifs.exceptions(std::ifstream::failbit);
+ std::string value;
+ try
+ {
+ ifs.open(opath.str());
+ ifs >> value;
+ }
+ catch (std::ios_base::failure& fail)
+ {
+ throw IpmiException(IPMI_CC_UNSPECIFIED_ERROR);
+ }
+
+ // If value parses as expected, return version.
+ VersionTuple version = std::make_tuple(0, 0, 0, 0);
+
+ int num_fields =
+ std::sscanf(value.c_str(), "%" SCNu8 ".%" SCNu8 ".%" SCNu8 ".%" SCNu8,
+ &std::get<0>(version), &std::get<1>(version),
+ &std::get<2>(version), &std::get<3>(version));
+ if (num_fields == 0)
+ {
+ std::fprintf(stderr, "Invalid version.\n");
+ throw IpmiException(IPMI_CC_UNSPECIFIED_ERROR);
+ }
+
+ return version;
+}
+
Handler handlerImpl;
} // namespace ipmi
diff --git a/handler.hpp b/handler.hpp
index e2e10fa..735ee15 100644
--- a/handler.hpp
+++ b/handler.hpp
@@ -9,6 +9,9 @@
namespace ipmi
{
+using VersionTuple =
+ std::tuple<std::uint8_t, std::uint8_t, std::uint8_t, std::uint8_t>;
+
class HandlerInterface
{
public:
@@ -29,6 +32,15 @@
* @throw IpmiException on failure.
*/
virtual std::int64_t getRxPackets(const std::string& name) const = 0;
+
+ /**
+ * Return the values from a cpld version file.
+ *
+ * @param[in] id - the cpld id number.
+ * @return the quad of numbers as a tuple (maj,min,pt,subpt)
+ * @throw IpmiException on failure.
+ */
+ virtual VersionTuple getCpldVersion(unsigned int id) const = 0;
};
class Handler : public HandlerInterface
@@ -39,6 +51,7 @@
std::tuple<std::uint8_t, std::string> getEthDetails() const override;
std::int64_t getRxPackets(const std::string& name) const override;
+ VersionTuple getCpldVersion(unsigned int id) const override;
};
extern Handler handlerImpl;
diff --git a/test/Makefile.am b/test/Makefile.am
index 90e42e1..df796d7 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -26,3 +26,7 @@
check_PROGRAMS += cable_unittest
cable_unittest_SOURCES = cable_unittest.cpp
cable_unittest_LDADD = $(top_builddir)/libsyscmds_common.la
+
+check_PROGRAMS += cpld_unittest
+cpld_unittest_SOURCES = cpld_unittest.cpp
+cpld_unittest_LDADD = $(top_builddir)/libsyscmds_common.la
diff --git a/test/cpld_unittest.cpp b/test/cpld_unittest.cpp
new file mode 100644
index 0000000..1c6caa3
--- /dev/null
+++ b/test/cpld_unittest.cpp
@@ -0,0 +1,54 @@
+#include "cpld.hpp"
+#include "handler_mock.hpp"
+#include "main.hpp"
+
+#include <cstdint>
+#include <tuple>
+#include <vector>
+
+#include <gtest/gtest.h>
+
+#define MAX_IPMI_BUFFER 64
+
+using ::testing::Return;
+
+namespace google
+{
+namespace ipmi
+{
+
+TEST(CpldCommandTest, RequestTooSmall)
+{
+ std::vector<std::uint8_t> request = {SysOEMCommands::SysCpldVersion};
+ size_t dataLen = request.size();
+ std::uint8_t reply[MAX_IPMI_BUFFER];
+
+ HandlerMock hMock;
+ EXPECT_EQ(IPMI_CC_REQ_DATA_LEN_INVALID,
+ CpldVersion(request.data(), reply, &dataLen, &hMock));
+}
+
+TEST(CpldCommandTest, ValidRequestReturnsHappy)
+{
+ std::vector<std::uint8_t> request = {SysOEMCommands::SysCpldVersion, 0x04};
+ size_t dataLen = request.size();
+ std::uint8_t reply[MAX_IPMI_BUFFER];
+ std::uint8_t expectedMaj = 0x5;
+ std::uint8_t expectedMin = 0x3;
+ std::uint8_t expectedPt = 0x7;
+ std::uint8_t expectedSbPtr = 0x9;
+
+ HandlerMock hMock;
+ EXPECT_CALL(hMock, getCpldVersion(0x04))
+ .WillOnce(Return(std::make_tuple(expectedMaj, expectedMin, expectedPt,
+ expectedSbPtr)));
+
+ EXPECT_EQ(IPMI_CC_OK, CpldVersion(request.data(), reply, &dataLen, &hMock));
+ EXPECT_EQ(expectedMaj, reply[1]);
+ EXPECT_EQ(expectedMin, reply[2]);
+ EXPECT_EQ(expectedPt, reply[3]);
+ EXPECT_EQ(expectedSbPtr, reply[4]);
+}
+
+} // namespace ipmi
+} // namespace google
diff --git a/test/handler_mock.hpp b/test/handler_mock.hpp
index a99f131..a92606a 100644
--- a/test/handler_mock.hpp
+++ b/test/handler_mock.hpp
@@ -17,6 +17,9 @@
MOCK_CONST_METHOD0(getEthDetails, std::tuple<std::uint8_t, std::string>());
MOCK_CONST_METHOD1(getRxPackets, std::int64_t(const std::string&));
+ MOCK_CONST_METHOD1(getCpldVersion,
+ std::tuple<std::uint8_t, std::uint8_t, std::uint8_t,
+ std::uint8_t>(unsigned int));
};
} // namespace ipmi