Break out DuplicatableFileHandle
Static analysis notes that writing non-trivial move constructors and
move operator= handlers is non trivial [1]. Funnily enough, we actually
already had bugs on this that were fixed in 06fc9be.
Simplify the code.
Tested: Redfish service validator passes.
[1] https://sonarcloud.io/project/issues?issues=AY9_HYiwKXKyw1ZFwgUG%2CAY9_HYiwKXKyw1ZFwgUF&id=edtanous_bmcweb&open=AY9_HYiwKXKyw1ZFwgUG
Change-Id: If96383d8c669ae9e5a8cc13304071b2dfe1ff2d2
Signed-off-by: Ed Tanous <ed@tanous.net>
diff --git a/http/http_body.hpp b/http/http_body.hpp
index 7be23f0..4462a50 100644
--- a/http/http_body.hpp
+++ b/http/http_body.hpp
@@ -1,5 +1,6 @@
#pragma once
+#include "duplicatable_file_handle.hpp"
#include "logging.hpp"
#include "utility.hpp"
@@ -33,57 +34,19 @@
class HttpBody::value_type
{
- boost::beast::file_posix fileHandle;
+ DuplicatableFileHandle fileHandle;
std::optional<size_t> fileSize;
std::string strBody;
public:
+ value_type() = default;
+ explicit value_type(std::string_view s) : strBody(s) {}
+ explicit value_type(EncodingType e) : encodingType(e) {}
EncodingType encodingType = EncodingType::Raw;
- ~value_type() = default;
- value_type() = default;
- explicit value_type(EncodingType enc) : encodingType(enc) {}
- explicit value_type(std::string_view str) : strBody(str) {}
-
- value_type(value_type&& other) noexcept :
- fileHandle(std::move(other.fileHandle)), fileSize(other.fileSize),
- strBody(std::move(other.strBody)), encodingType(other.encodingType)
- {}
-
- value_type& operator=(value_type&& other) noexcept
+ const boost::beast::file_posix& file() const
{
- fileHandle = std::move(other.fileHandle);
- fileSize = other.fileSize;
- strBody = std::move(other.strBody);
- encodingType = other.encodingType;
-
- return *this;
- }
-
- // Overload copy constructor, because posix doesn't have dup(), but linux
- // does
- value_type(const value_type& other) :
- fileSize(other.fileSize), strBody(other.strBody),
- encodingType(other.encodingType)
- {
- fileHandle.native_handle(dup(other.fileHandle.native_handle()));
- }
-
- value_type& operator=(const value_type& other)
- {
- if (this != &other)
- {
- fileSize = other.fileSize;
- strBody = other.strBody;
- encodingType = other.encodingType;
- fileHandle.native_handle(dup(other.fileHandle.native_handle()));
- }
- return *this;
- }
-
- const boost::beast::file_posix& file()
- {
- return fileHandle;
+ return fileHandle.fileHandle;
}
std::string& str()
@@ -98,7 +61,7 @@
std::optional<size_t> payloadSize() const
{
- if (!fileHandle.is_open())
+ if (!fileHandle.fileHandle.is_open())
{
return strBody.size();
}
@@ -116,7 +79,7 @@
{
strBody.clear();
strBody.shrink_to_fit();
- fileHandle = boost::beast::file_posix();
+ fileHandle.fileHandle = boost::beast::file_posix();
fileSize = std::nullopt;
encodingType = EncodingType::Raw;
}
@@ -124,13 +87,13 @@
void open(const char* path, boost::beast::file_mode mode,
boost::system::error_code& ec)
{
- fileHandle.open(path, mode, ec);
+ fileHandle.fileHandle.open(path, mode, ec);
if (ec)
{
return;
}
boost::system::error_code ec2;
- uint64_t size = fileHandle.size(ec2);
+ uint64_t size = fileHandle.fileHandle.size(ec2);
if (!ec2)
{
BMCWEB_LOG_INFO("File size was {} bytes", size);
@@ -141,7 +104,7 @@
BMCWEB_LOG_WARNING("Failed to read file size on {}", path);
}
- int fadvise = posix_fadvise(fileHandle.native_handle(), 0, 0,
+ int fadvise = posix_fadvise(fileHandle.fileHandle.native_handle(), 0, 0,
POSIX_FADV_SEQUENTIAL);
if (fadvise != 0)
{
@@ -152,10 +115,10 @@
void setFd(int fd, boost::system::error_code& ec)
{
- fileHandle.native_handle(fd);
+ fileHandle.fileHandle.native_handle(fd);
boost::system::error_code ec2;
- uint64_t size = fileHandle.size(ec2);
+ uint64_t size = fileHandle.fileHandle.size(ec2);
if (!ec2)
{
if (size != 0 && size < std::numeric_limits<size_t>::max())
diff --git a/include/duplicatable_file_handle.hpp b/include/duplicatable_file_handle.hpp
new file mode 100644
index 0000000..6d2514e
--- /dev/null
+++ b/include/duplicatable_file_handle.hpp
@@ -0,0 +1,27 @@
+#include <boost/beast/core/file_posix.hpp>
+
+struct DuplicatableFileHandle
+{
+ boost::beast::file_posix fileHandle;
+
+ DuplicatableFileHandle() = default;
+ DuplicatableFileHandle(DuplicatableFileHandle&&) noexcept = default;
+ // Overload copy constructor, because posix doesn't have dup(), but linux
+ // does
+ DuplicatableFileHandle(const DuplicatableFileHandle& other)
+ {
+ fileHandle.native_handle(dup(other.fileHandle.native_handle()));
+ }
+ DuplicatableFileHandle& operator=(const DuplicatableFileHandle& other)
+ {
+ if (this == &other)
+ {
+ return *this;
+ }
+ fileHandle.native_handle(dup(other.fileHandle.native_handle()));
+ return *this;
+ }
+ DuplicatableFileHandle&
+ operator=(DuplicatableFileHandle&& other) noexcept = default;
+ ~DuplicatableFileHandle() = default;
+};