diff options
-rw-r--r-- | include/ipmid/handler.hpp | 1 | ||||
-rw-r--r-- | include/ipmid/message.hpp | 10 | ||||
-rw-r--r-- | include/ipmid/message/unpack.hpp | 6 | ||||
-rw-r--r-- | test/message/payload.cpp | 119 |
4 files changed, 125 insertions, 11 deletions
diff --git a/include/ipmid/handler.hpp b/include/ipmid/handler.hpp index 1421c3d..ebeb442 100644 --- a/include/ipmid/handler.hpp +++ b/include/ipmid/handler.hpp @@ -153,6 +153,7 @@ class IpmiHandler final : public HandlerBase using ResultType = boost::callable_traits::return_type_t<Handler>; UnpackArgsType unpackArgs; + request->payload.trailingOk = false; ipmi::Cc unpackError = request->unpack(unpackArgs); if (unpackError != ipmi::ccSuccess) { diff --git a/include/ipmid/message.hpp b/include/ipmid/message.hpp index 7f1ad7e..b10fc42 100644 --- a/include/ipmid/message.hpp +++ b/include/ipmid/message.hpp @@ -103,15 +103,14 @@ struct Payload Payload(Payload&&) = default; Payload& operator=(Payload&&) = default; - explicit Payload(std::vector<uint8_t>&& data) : - raw(std::move(data)), unpackCheck(false) + explicit Payload(std::vector<uint8_t>&& data) : raw(std::move(data)) { } ~Payload() { using namespace phosphor::logging; - if (trailingOk && !unpackCheck && !fullyUnpacked()) + if (raw.size() != 0 && !trailingOk && !unpackCheck && !unpackError) { log<level::ERR>("Failed to check request for full unpack"); } @@ -449,8 +448,8 @@ struct Payload size_t bitCount = 0; std::vector<uint8_t> raw; size_t rawIndex = 0; - bool trailingOk = false; - bool unpackCheck = true; + bool trailingOk = true; + bool unpackCheck = false; bool unpackError = false; }; @@ -573,7 +572,6 @@ struct Request // not all bits were consumed by requested parameters return ipmi::ccReqDataLenInvalid; } - payload.unpackCheck = false; } } return unpackRet; diff --git a/include/ipmid/message/unpack.hpp b/include/ipmid/message/unpack.hpp index 94f80f1..fb2b066 100644 --- a/include/ipmid/message/unpack.hpp +++ b/include/ipmid/message/unpack.hpp @@ -324,13 +324,9 @@ struct UnpackSingle<Payload> { static int op(Payload& p, Payload& t) { + t = p; // mark that this payload is being included in the args p.trailingOk = true; - t = p; - // reset the unpacking flags so it can be properly checked - t.trailingOk = false; - t.unpackCheck = true; - t.unpackError = false; return 0; } }; diff --git a/test/message/payload.cpp b/test/message/payload.cpp index 9d20ff0..431a34b 100644 --- a/test/message/payload.cpp +++ b/test/message/payload.cpp @@ -13,6 +13,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +#define SD_JOURNAL_SUPPRESS_LOCATION + +#include <systemd/sd-journal.h> + #include <ipmid/api.hpp> #include <ipmid/message.hpp> @@ -306,3 +310,118 @@ TEST(PayloadRequest, PartialPayload) uint32_t k2 = 0x02008604; ASSERT_EQ(v2, k2); } + +std::vector<std::string> logs; + +extern "C" { +int sd_journal_send(const char* format, ...) +{ + logs.push_back(format); + return 0; +} + +int sd_journal_send_with_location(const char* file, const char* line, + const char* func, const char* format, ...) +{ + logs.push_back(format); + return 0; +} +} + +class PayloadLogging : public testing::Test +{ + public: + void SetUp() + { + logs.clear(); + } +}; + +TEST_F(PayloadLogging, TrailingOk) +{ + { + ipmi::message::Payload p({1, 2}); + } + EXPECT_EQ(logs.size(), 0); +} + +TEST_F(PayloadLogging, EnforcingUnchecked) +{ + { + ipmi::message::Payload p({1, 2}); + p.trailingOk = false; + } + EXPECT_EQ(logs.size(), 1); +} + +TEST_F(PayloadLogging, EnforcingUncheckedUnpacked) +{ + { + ipmi::message::Payload p({1, 2}); + p.trailingOk = false; + uint8_t out; + p.unpack(out, out); + } + EXPECT_EQ(logs.size(), 1); +} + +TEST_F(PayloadLogging, EnforcingUncheckedError) +{ + { + ipmi::message::Payload p({1, 2}); + p.trailingOk = false; + uint32_t out; + p.unpack(out); + } + EXPECT_EQ(logs.size(), 0); +} + +TEST_F(PayloadLogging, EnforcingChecked) +{ + { + ipmi::message::Payload p({1, 2}); + p.trailingOk = false; + EXPECT_FALSE(p.fullyUnpacked()); + } + EXPECT_EQ(logs.size(), 0); +} + +TEST_F(PayloadLogging, EnforcingCheckedUnpacked) +{ + { + ipmi::message::Payload p({1, 2}); + p.trailingOk = false; + uint8_t out; + p.unpack(out, out); + EXPECT_TRUE(p.fullyUnpacked()); + } + EXPECT_EQ(logs.size(), 0); +} + +TEST_F(PayloadLogging, EnforcingUnpackPayload) +{ + { + ipmi::message::Payload p; + { + ipmi::message::Payload q({1, 2}); + q.trailingOk = false; + q.unpack(p); + } + EXPECT_EQ(logs.size(), 0); + } + EXPECT_EQ(logs.size(), 1); +} + +TEST_F(PayloadLogging, EnforcingMove) +{ + { + ipmi::message::Payload p; + { + ipmi::message::Payload q({1, 2}); + q.trailingOk = false; + p = std::move(q); + } + EXPECT_EQ(logs.size(), 0); + } + EXPECT_EQ(logs.size(), 1); +} |