Not passing fd in the zero and pattern erase
The original erase zero, and erase pattern methods took fd that were
opened by the client code. This changes make zero and pattern open the
files internally, which is safe and a more standard practice.
Tested: This change has not been tested
Change-Id: Iae848429b304aa39d510d1f090ace416db88ac7e
Signed-off-by: John Edward Broadbent <jebr@google.com>
diff --git a/include/pattern.hpp b/include/pattern.hpp
index 8c5b7d1..b64eff7 100644
--- a/include/pattern.hpp
+++ b/include/pattern.hpp
@@ -24,15 +24,13 @@
* and throws errors accordingly.
*
* @param[in] bytes - Size of the block device
- * @param[in] managedFd - the file descriptor for the open drive
*/
- void writePattern(uint64_t driveSize, ManagedFd& fd);
+ void writePattern(uint64_t driveSize);
/** @brief verifies the uncompressible random pattern is on the drive
* and throws errors accordingly.
*
* @param[in] bytes - Size of the block device
- * @param[in] managedFd - the file descriptor for the open drive
*/
- void verifyPattern(uint64_t driveSize, ManagedFd& fd);
+ void verifyPattern(uint64_t driveSize);
};
diff --git a/include/zero.hpp b/include/zero.hpp
index b6ca93f..e8675e8 100644
--- a/include/zero.hpp
+++ b/include/zero.hpp
@@ -24,17 +24,15 @@
* and throws errors accordingly.
*
* @param[in] bytes - Size of the block device
- * @param[in] managedFd - the file descriptor for the open drive
*/
- void writeZero(uint64_t driveSize, ManagedFd& fd);
+ void writeZero(uint64_t driveSize);
/** @brief verifies the uncompressible random pattern is on the drive
* and throws errors accordingly.
*
* @param[in] bytes - Size of the block device
- * @param[in] managedFd - the file descriptor for the open drive
*/
- void verifyZero(uint64_t driveSize, ManagedFd& fd);
+ void verifyZero(uint64_t driveSize);
private:
/* @brief the size of the blocks in bytes used for write and verify.
diff --git a/src/erase/pattern.cpp b/src/erase/pattern.cpp
index 649bd37..6fb08a7 100644
--- a/src/erase/pattern.cpp
+++ b/src/erase/pattern.cpp
@@ -22,13 +22,17 @@
using sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
using stdplus::fd::ManagedFd;
-void Pattern::writePattern(const uint64_t driveSize, ManagedFd& fd)
+void Pattern::writePattern(const uint64_t driveSize)
{
// static seed defines a fixed prng sequnce so it can be verified later,
// and validated for entropy
uint64_t currentIndex = 0;
std::minstd_rand0 generator(seed);
std::array<std::byte, blockSize> randArr;
+
+ ManagedFd fd =
+ stdplus::fd::open(devPath, stdplus::fd::OpenAccess::WriteOnly);
+
while (currentIndex < driveSize)
{
// generate a 4k block of prng
@@ -54,13 +58,17 @@
}
}
-void Pattern::verifyPattern(const uint64_t driveSize, ManagedFd& fd)
+void Pattern::verifyPattern(const uint64_t driveSize)
{
uint64_t currentIndex = 0;
std::minstd_rand0 generator(seed);
std::array<std::byte, blockSize> randArr;
std::array<std::byte, blockSize> readArr;
+
+ ManagedFd fd =
+ stdplus::fd::open(devPath, stdplus::fd::OpenAccess::ReadOnly);
+
while (currentIndex < driveSize)
{
size_t readSize = currentIndex + blockSize < driveSize
diff --git a/src/erase/zero.cpp b/src/erase/zero.cpp
index accb8c8..2c0d615 100644
--- a/src/erase/zero.cpp
+++ b/src/erase/zero.cpp
@@ -16,9 +16,12 @@
using sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
using stdplus::fd::ManagedFd;
-void Zero::writeZero(const uint64_t driveSize, ManagedFd& fd)
+void Zero::writeZero(const uint64_t driveSize)
{
+ ManagedFd fd =
+ stdplus::fd::open(devPath, stdplus::fd::OpenAccess::WriteOnly);
+
uint64_t currentIndex = 0;
const std::array<const std::byte, blockSize> blockOfZeros{};
@@ -42,8 +45,11 @@
}
}
-void Zero::verifyZero(uint64_t driveSize, ManagedFd& fd)
+void Zero::verifyZero(uint64_t driveSize)
{
+ ManagedFd fd =
+ stdplus::fd::open(devPath, stdplus::fd::OpenAccess::ReadOnly);
+
uint64_t currentIndex = 0;
std::array<std::byte, blockSize> readArr;
const std::array<const std::byte, blockSize> blockOfZeros{};
diff --git a/src/estoraged.cpp b/src/estoraged.cpp
index cb8f0fa..3e3d0bb 100644
--- a/src/estoraged.cpp
+++ b/src/estoraged.cpp
@@ -73,19 +73,14 @@
case EraseMethod::LogicalOverWrite:
{
Pattern myErasePattern(devPath);
- ManagedFd drivefd =
- stdplus::fd::open(devPath, stdplus::fd::OpenAccess::WriteOnly);
- myErasePattern.writePattern(myErasePattern.findSizeOfBlockDevice(),
- drivefd);
+ myErasePattern.writePattern(myErasePattern.findSizeOfBlockDevice());
break;
}
case EraseMethod::LogicalVerify:
{
Pattern myErasePattern(devPath);
- ManagedFd drivefd =
- stdplus::fd::open(devPath, stdplus::fd::OpenAccess::ReadOnly);
- myErasePattern.verifyPattern(myErasePattern.findSizeOfBlockDevice(),
- drivefd);
+ myErasePattern.verifyPattern(
+ myErasePattern.findSizeOfBlockDevice());
break;
}
case EraseMethod::VendorSanitize:
@@ -95,17 +90,13 @@
case EraseMethod::ZeroOverWrite:
{
Zero myZero(devPath);
- ManagedFd drivefd =
- stdplus::fd::open(devPath, stdplus::fd::OpenAccess::WriteOnly);
- myZero.writeZero(myZero.findSizeOfBlockDevice(), drivefd);
+ myZero.writeZero(myZero.findSizeOfBlockDevice());
break;
}
case EraseMethod::ZeroVerify:
{
Zero myZero(devPath);
- ManagedFd drivefd =
- stdplus::fd::open(devPath, stdplus::fd::OpenAccess::ReadOnly);
- myZero.verifyZero(myZero.findSizeOfBlockDevice(), drivefd);
+ myZero.verifyZero(myZero.findSizeOfBlockDevice());
break;
}
case EraseMethod::SecuredLocked:
diff --git a/src/test/erase/pattern_test.cpp b/src/test/erase/pattern_test.cpp
index 55e83a5..358f873 100644
--- a/src/test/erase/pattern_test.cpp
+++ b/src/test/erase/pattern_test.cpp
@@ -8,6 +8,7 @@
#include <stdplus/fd/managed.hpp>
#include <xyz/openbmc_project/Common/error.hpp>
+#include <fstream>
#include <system_error>
#include <gmock/gmock-matchers.h>
@@ -15,67 +16,52 @@
#include <gtest/gtest.h>
using sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
-using stdplus::fd::ManagedFd;
-
-class MockManagedFd : public ManagedFd
-{
- public:
- MockManagedFd(int fd) : ManagedFd(std::move(fd))
- {}
- ~MockManagedFd()
- {}
-};
TEST(pattern, patternPass)
{
+ std::string testFileName = "patternPass";
uint64_t size = 4096;
- int fds[2];
- Pattern pass("fileName");
- if (pipe(fds))
- {
- FAIL();
- }
- MockManagedFd writeFd(fds[1]);
- EXPECT_NO_THROW(pass.writePattern(size, writeFd));
- MockManagedFd verifyFd(fds[0]);
- EXPECT_NO_THROW(pass.verifyPattern(size, verifyFd));
+ std::ofstream testFile;
+ testFile.open(testFileName,
+ std::ios::out | std::ios::binary | std::ios::trunc);
+ testFile.close();
+
+ Pattern pass(testFileName);
+ EXPECT_NO_THROW(pass.writePattern(size));
+ EXPECT_NO_THROW(pass.verifyPattern(size));
}
/* This test that pattern writes the correct number of bytes even if
* size of the drive is not divisable by the block size
*/
-TEST(pattern, patternPassNotDivisible)
+TEST(pattern, patternNotDivisible)
{
+ std::string testFileName = "notDivisible";
uint64_t size = 4097;
// 4097 is not divisible by the block size, and we expect no errors
- int fds[2];
- Pattern pass("fileName");
- if (pipe(fds))
- {
- FAIL();
- }
- MockManagedFd writeFd(fds[1]);
- EXPECT_NO_THROW(pass.writePattern(size, writeFd));
- MockManagedFd verifyFd(fds[0]);
- EXPECT_NO_THROW(pass.verifyPattern(size, verifyFd));
+ std::ofstream testFile;
+ testFile.open(testFileName,
+ std::ios::out | std::ios::binary | std::ios::trunc);
+ testFile.close();
+
+ Pattern pass(testFileName);
+ EXPECT_NO_THROW(pass.writePattern(size));
+ EXPECT_NO_THROW(pass.verifyPattern(size));
}
TEST(pattern, patternsDontMatch)
{
+ std::string testFileName = "patternsDontMatch";
uint64_t size = 4096;
- int fds[2];
- Pattern pass("fileName");
- if (pipe(fds))
- {
- FAIL();
- }
+ std::ofstream testFile;
+
+ Pattern pass(testFileName);
+
int dummyValue = 88;
- if (::write(fds[1], &dummyValue, sizeof(dummyValue)) != sizeof(dummyValue))
- {
- FAIL();
- }
- MockManagedFd writeFd(fds[1]);
- pass.writePattern(size - sizeof(dummyValue), writeFd);
- MockManagedFd verifyFd(fds[0]);
- EXPECT_THROW(pass.verifyPattern(size, verifyFd), InternalFailure);
+ testFile.open(testFileName, std::ios::binary | std::ios::out);
+ testFile.write((const char*)&dummyValue, sizeof(dummyValue));
+ testFile.close();
+
+ EXPECT_NO_THROW(pass.writePattern(size - sizeof(dummyValue)));
+ EXPECT_THROW(pass.verifyPattern(size), InternalFailure);
}
diff --git a/src/test/erase/zero_test.cpp b/src/test/erase/zero_test.cpp
index b3b91ec..70a62e2 100644
--- a/src/test/erase/zero_test.cpp
+++ b/src/test/erase/zero_test.cpp
@@ -8,6 +8,7 @@
#include <stdplus/fd/managed.hpp>
#include <xyz/openbmc_project/Common/error.hpp>
+#include <fstream>
#include <system_error>
#include <gmock/gmock-matchers.h>
@@ -23,17 +24,16 @@
TEST(Zeros, zeroPass)
{
+ std::string testFileName = "testfile_pass";
+ std::ofstream testFile;
+
+ testFile.open(testFileName,
+ std::ios::out | std::ios::binary | std::ios::trunc);
+ testFile.close();
uint64_t size = 4096;
- int fds[2];
- Zero pass("fileName");
- if (pipe(fds))
- {
- FAIL();
- }
- ManagedFd writeFd(std::move(fds[1]));
- EXPECT_NO_THROW(pass.writeZero(size, writeFd));
- ManagedFd verifyFd(std::move(fds[0]));
- EXPECT_NO_THROW(pass.verifyZero(size, verifyFd));
+ Zero pass(testFileName);
+ EXPECT_NO_THROW(pass.writeZero(size));
+ EXPECT_NO_THROW(pass.verifyZero(size));
}
/* This test that pattern writes the correct number of bytes even if
@@ -41,59 +41,62 @@
*/
TEST(Zeros, notDivisible)
{
+ std::string testFileName = "testfile_notDivisible";
+ std::ofstream testFile;
+
+ testFile.open(testFileName,
+ std::ios::out | std::ios::binary | std::ios::trunc);
+ testFile.close();
+
uint64_t size = 4097;
// 4097 is not divisible by the block size, and we expect no errors
- int fds[2];
- Zero pass("fileName");
- if (pipe(fds))
- {
- FAIL();
- }
- ManagedFd writeFd(std::move(fds[1]));
- EXPECT_NO_THROW(pass.writeZero(size, writeFd));
- ManagedFd verifyFd(std::move(fds[0]));
- EXPECT_NO_THROW(pass.verifyZero(size, verifyFd));
+ Zero pass(testFileName);
+ EXPECT_NO_THROW(pass.writeZero(size));
+ EXPECT_NO_THROW(pass.verifyZero(size));
}
TEST(Zeros, notZeroStart)
{
+ std::string testFileName = "testfile_notZeroStart";
+ std::ofstream testFile;
+
+ // open the file and write none zero to it
+ int dummyValue = 0x88776655;
+ testFile.open(testFileName, std::ios::binary | std::ios::out);
+ testFile.write((const char*)&dummyValue, sizeof(dummyValue));
+ testFile.close();
uint64_t size = 4096;
- int fds[2];
- Zero pass("fileName");
- if (pipe(fds))
- {
- FAIL();
- }
- int dummyValue = 88;
- if (::write(fds[1], &dummyValue, sizeof(dummyValue)) != sizeof(dummyValue))
- {
- FAIL();
- }
- ManagedFd writeFd(std::move(fds[1]));
- EXPECT_NO_THROW(pass.writeZero(size - sizeof(dummyValue), writeFd));
- ManagedFd verifyFd(std::move(fds[0]));
- EXPECT_THROW(pass.verifyZero(size, verifyFd), InternalFailure);
+ Zero pass(testFileName);
+ EXPECT_NO_THROW(pass.writeZero(size - sizeof(dummyValue)));
+
+ // force flush, needed for unit testing
+ std::ofstream file;
+ file.open(testFileName);
+ file.close();
+
+ EXPECT_THROW(pass.verifyZero(size), InternalFailure);
}
TEST(Zeros, notZeroEnd)
{
+ std::string testFileName = "testfile_notZeroEnd";
+ std::ofstream testFile;
+
+ testFile.open(testFileName,
+ std::ios::out | std::ios::binary | std::ios::trunc);
+ testFile.close();
+
uint64_t size = 4096;
- int fds[2];
- Zero pass("fileName");
- if (pipe(fds))
- {
- FAIL();
- }
+ Zero pass(testFileName);
int dummyValue = 88;
- int tmpFd = fds[1];
- ManagedFd writeFd(std::move(tmpFd));
- EXPECT_NO_THROW(pass.writeZero(size - sizeof(dummyValue), writeFd));
- if (::write(fds[1], &dummyValue, sizeof(dummyValue)) != sizeof(dummyValue))
- {
- FAIL();
- }
- ManagedFd verifyFd(std::move(fds[0]));
- EXPECT_THROW(pass.verifyZero(size, verifyFd), InternalFailure);
+ EXPECT_NO_THROW(pass.writeZero(size - sizeof(dummyValue)));
+
+ // open the file and write none zero to it
+ testFile.open(testFileName, std::ios::out);
+ testFile << dummyValue;
+ testFile.close();
+
+ EXPECT_THROW(pass.verifyZero(size), InternalFailure);
}
} // namespace estoraged_test