tools/helper: Streamline return types
These functions can already return exceptions, so make better use of
them for reporting failures.
Change-Id: I572e9e3ee44bbd5add601f3246bb4f95cb9308bf
Signed-off-by: William A. Kennington III <wak@google.com>
diff --git a/tools/handler.cpp b/tools/handler.cpp
index a87793e..2b847a9 100644
--- a/tools/handler.cpp
+++ b/tools/handler.cpp
@@ -115,7 +115,8 @@
std::fprintf(stderr, "Calling stat on %s session to check status\n",
target.c_str());
- return pollStatus(*session, blob);
+ pollStatus(*session, blob);
+ return true;
}
catch (const ipmiblob::BlobException& b)
{
@@ -139,10 +140,10 @@
/* TODO: call readBytes multiple times in case IPMI message length
* exceeds IPMI_MAX_MSG_LENGTH.
*/
- auto pollResp = pollReadReady(*session, blob);
- if (pollResp.first && pollResp.second > 0)
+ auto size = pollReadReady(*session, blob);
+ if (size > 0)
{
- return blob->readBytes(*session, 0, pollResp.second);
+ return blob->readBytes(*session, 0, size);
}
return {};
}
diff --git a/tools/helper.cpp b/tools/helper.cpp
index d612922..7ae8c76 100644
--- a/tools/helper.cpp
+++ b/tools/helper.cpp
@@ -32,7 +32,7 @@
/* Poll an open verification session. Handling closing the session is not yet
* owned by this method.
*/
-bool pollStatus(std::uint16_t session, ipmiblob::BlobInterface* blob)
+void pollStatus(std::uint16_t session, ipmiblob::BlobInterface* blob)
{
using namespace std::chrono_literals;
@@ -109,7 +109,10 @@
* which exceptions from the lower layers allow one to try and delete the
* blobs to rollback the state and progress.
*/
- return (result == ipmi_flash::ActionStatus::success);
+ if (result != ipmi_flash::ActionStatus::success)
+ {
+ throw ToolException("BMC reported failure");
+ }
}
/* Poll an open blob session for reading.
@@ -126,8 +129,7 @@
* If the blob is not open_read and not committing, then it is an error to the
* reader.
*/
-std::pair<bool, uint32_t> pollReadReady(std::uint16_t session,
- ipmiblob::BlobInterface* blob)
+uint32_t pollReadReady(std::uint16_t session, ipmiblob::BlobInterface* blob)
{
using namespace std::chrono_literals;
static constexpr auto pollingSleep = 5s;
@@ -149,7 +151,7 @@
if (blobStatResp.blob_state & ipmiblob::StateFlags::open_read)
{
std::fprintf(stderr, "success\n");
- return std::make_pair(true, blobStatResp.size);
+ return blobStatResp.size;
}
else if (blobStatResp.blob_state & ipmiblob::StateFlags::committing)
{
@@ -158,7 +160,7 @@
else
{
std::fprintf(stderr, "failed\n");
- return std::make_pair(false, 0);
+ throw ToolException("BMC reported failure");
}
std::this_thread::sleep_for(pollingSleep);
@@ -170,7 +172,7 @@
std::string(b.what()));
}
- return std::make_pair(false, 0);
+ throw ToolException("Timed out waiting for BMC read ready");
}
void* memcpyAligned(void* destination, const void* source, std::size_t size)
diff --git a/tools/helper.hpp b/tools/helper.hpp
index 9b987ac..ebe7a1b 100644
--- a/tools/helper.hpp
+++ b/tools/helper.hpp
@@ -14,7 +14,7 @@
* @param[in] blob - pointer to blob interface implementation object.
* @return true if the verification was successful.
*/
-bool pollStatus(std::uint16_t session, ipmiblob::BlobInterface* blob);
+void pollStatus(std::uint16_t session, ipmiblob::BlobInterface* blob);
/**
* Poll an open firmware version blob session and check if it ready to read.
@@ -23,8 +23,7 @@
* @param[in] blob - pointer to blob interface implementation object
* @return the polling status and blob buffer size
*/
-std::pair<bool, uint32_t> pollReadReady(std::uint16_t session,
- ipmiblob::BlobInterface* blob);
+uint32_t pollReadReady(std::uint16_t session, ipmiblob::BlobInterface* blob);
/**
* Aligned memcpy
diff --git a/tools/test/tools_helper_unittest.cpp b/tools/test/tools_helper_unittest.cpp
index 2bee66d..cfe5208 100644
--- a/tools/test/tools_helper_unittest.cpp
+++ b/tools/test/tools_helper_unittest.cpp
@@ -1,5 +1,6 @@
#include "helper.hpp"
#include "status.hpp"
+#include "tool_errors.hpp"
#include <blobs-ipmid/blobs.hpp>
#include <ipmiblob/test/blob_interface_mock.hpp>
@@ -30,7 +31,7 @@
EXPECT_CALL(blobMock, getStat(TypedEq<std::uint16_t>(session)))
.WillOnce(Return(verificationResponse));
- EXPECT_TRUE(pollStatus(session, &blobMock));
+ EXPECT_NO_THROW(pollStatus(session, &blobMock));
}
TEST_F(HelperTest, PollStatusReturnsAfterFailure)
@@ -43,7 +44,7 @@
EXPECT_CALL(blobMock, getStat(TypedEq<std::uint16_t>(session)))
.WillOnce(Return(verificationResponse));
- EXPECT_FALSE(pollStatus(session, &blobMock));
+ EXPECT_THROW(pollStatus(session, &blobMock), ToolException);
}
TEST_F(HelperTest, PollReadReadyReturnsAfterSuccess)
@@ -56,7 +57,7 @@
EXPECT_CALL(blobMock, getStat(TypedEq<std::uint16_t>(session)))
.WillOnce(Return(blobResponse));
- EXPECT_TRUE(pollReadReady(session, &blobMock).first);
+ EXPECT_NO_THROW(pollReadReady(session, &blobMock));
}
TEST_F(HelperTest, PollReadReadyReturnsAfterFailure)
@@ -68,7 +69,7 @@
EXPECT_CALL(blobMock, getStat(TypedEq<std::uint16_t>(session)))
.WillOnce(Return(blobResponse));
- EXPECT_FALSE(pollReadReady(session, &blobMock).first);
+ EXPECT_THROW(pollReadReady(session, &blobMock), ToolException);
}
TEST_F(HelperTest, PollReadReadyReturnsAfterRetrySuccess)
@@ -85,7 +86,7 @@
.WillOnce(Return(blobResponseRunning))
.WillOnce(Return(blobResponseReadReady));
- EXPECT_TRUE(pollReadReady(session, &blobMock).first);
+ EXPECT_NO_THROW(pollReadReady(session, &blobMock));
}
TEST_F(HelperTest, PollReadReadyReturnsAfterRetryFailure)
@@ -102,7 +103,7 @@
.WillOnce(Return(blobResponseRunning))
.WillOnce(Return(blobResponseError));
- EXPECT_FALSE(pollReadReady(session, &blobMock).first);
+ EXPECT_THROW(pollReadReady(session, &blobMock), ToolException);
}
TEST_F(HelperTest, MemcpyAlignedOneByte)
diff --git a/tools/test/tools_updater_unittest.cpp b/tools/test/tools_updater_unittest.cpp
index 60cc077..40b6798 100644
--- a/tools/test/tools_updater_unittest.cpp
+++ b/tools/test/tools_updater_unittest.cpp
@@ -210,9 +210,9 @@
ToolException);
}
-TEST_F(UpdateHandlerTest, ReadVersionReturnsEmptyIfPollingFails)
+TEST_F(UpdateHandlerTest, ReadVersionReturnsErrorIfPollingFails)
{
- /* It can return an empty result, when polling fails. */
+ /* It can throw an error, when polling fails. */
EXPECT_CALL(blobMock, openBlob(ipmi_flash::biosVersionBlobId, _))
.WillOnce(Return(session));
ipmiblob::StatResponse readVersionResponse = {};
@@ -220,7 +220,8 @@
EXPECT_CALL(blobMock, getStat(TypedEq<std::uint16_t>(session)))
.WillOnce(Return(readVersionResponse));
EXPECT_CALL(blobMock, closeBlob(session)).WillOnce(Return());
- EXPECT_THAT(updater.readVersion(ipmi_flash::biosVersionBlobId), IsEmpty());
+ EXPECT_THROW(updater.readVersion(ipmi_flash::biosVersionBlobId),
+ ToolException);
}
TEST_F(UpdateHandlerTest, ReadVersionCovertsOpenBlobExceptionToToolException)