diff --git a/sdk/storage/azure-storage-common/inc/azure/storage/common/internal/xml_wrapper.hpp b/sdk/storage/azure-storage-common/inc/azure/storage/common/internal/xml_wrapper.hpp index 82b6ea2c73..093c32fcf1 100644 --- a/sdk/storage/azure-storage-common/inc/azure/storage/common/internal/xml_wrapper.hpp +++ b/sdk/storage/azure-storage-common/inc/azure/storage/common/internal/xml_wrapper.hpp @@ -4,6 +4,7 @@ #pragma once #include +#include #include namespace Azure { namespace Storage { namespace _internal { @@ -40,19 +41,15 @@ namespace Azure { namespace Storage { namespace _internal { explicit XmlReader(const char* data, size_t length); XmlReader(const XmlReader& other) = delete; XmlReader& operator=(const XmlReader& other) = delete; - XmlReader(XmlReader&& other) noexcept { *this = std::move(other); } - XmlReader& operator=(XmlReader&& other) noexcept - { - m_context = other.m_context; - other.m_context = nullptr; - return *this; - } + XmlReader(XmlReader&& other) noexcept; + XmlReader& operator=(XmlReader&& other) noexcept; ~XmlReader(); XmlNode Read(); private: - void* m_context = nullptr; + struct XmlReaderContext; + std::unique_ptr m_context; }; class XmlWriter final { @@ -60,13 +57,8 @@ namespace Azure { namespace Storage { namespace _internal { explicit XmlWriter(); XmlWriter(const XmlWriter& other) = delete; XmlWriter& operator=(const XmlWriter& other) = delete; - XmlWriter(XmlWriter&& other) noexcept { *this = std::move(other); } - XmlWriter& operator=(XmlWriter&& other) noexcept - { - m_context = other.m_context; - other.m_context = nullptr; - return *this; - } + XmlWriter(XmlWriter&& other) noexcept; + XmlWriter& operator=(XmlWriter&& other) noexcept; ~XmlWriter(); void Write(XmlNode node); @@ -74,7 +66,8 @@ namespace Azure { namespace Storage { namespace _internal { std::string GetDocument(); private: - void* m_context = nullptr; + struct XmlWriterContext; + std::unique_ptr m_context; }; }}} // namespace Azure::Storage::_internal diff --git a/sdk/storage/azure-storage-common/src/xml_wrapper.cpp b/sdk/storage/azure-storage-common/src/xml_wrapper.cpp index 3af59fa0e9..435d631b8a 100644 --- a/sdk/storage/azure-storage-common/src/xml_wrapper.cpp +++ b/sdk/storage/azure-storage-common/src/xml_wrapper.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #if defined(AZ_PLATFORM_WINDOWS) @@ -27,7 +28,10 @@ namespace Azure { namespace Storage { namespace _internal { #if defined(AZ_PLATFORM_WINDOWS) - struct XmlReaderContext + void XmlGlobalInitialize() {} + void XmlGlobalDeinitialize() {} + + struct XmlReader::XmlReaderContext { XmlReaderContext() { @@ -95,20 +99,14 @@ namespace Azure { namespace Storage { namespace _internal { throw std::runtime_error("Unsupported xml encoding."); } - m_context = context.release(); + m_context = std::move(context); } - XmlReader::~XmlReader() - { - if (m_context) - { - delete static_cast(m_context); - } - } + XmlReader::~XmlReader() {} XmlNode XmlReader::Read() { - auto context = static_cast(m_context); + auto& context = m_context; auto moveToNext = [&]() { HRESULT ret = WsReadNode(context->reader, context->error); @@ -218,7 +216,7 @@ namespace Azure { namespace Storage { namespace _internal { } } - struct XmlWriterContext + struct XmlWriter::XmlWriterContext { XmlWriterContext() { @@ -272,20 +270,14 @@ namespace Azure { namespace Storage { namespace _internal { throw std::runtime_error("Failed to initialize xml writer."); } - m_context = context.release(); + m_context = std::move(context); } - XmlWriter::~XmlWriter() - { - if (m_context) - { - delete static_cast(m_context); - } - } + XmlWriter::~XmlWriter() {} void XmlWriter::Write(XmlNode node) { - auto context = static_cast(m_context); + auto& context = m_context; if (node.Type == XmlNodeType::StartTag) { if (node.HasValue) @@ -359,7 +351,7 @@ namespace Azure { namespace Storage { namespace _internal { std::string XmlWriter::GetDocument() { - auto context = static_cast(m_context); + auto& context = m_context; BOOL boolValueTrue = TRUE; WS_XML_WRITER_PROPERTY writerProperty[2]; @@ -400,11 +392,15 @@ namespace Azure { namespace Storage { namespace _internal { static void XmlGlobalInitialize() { static XmlGlobalInitializer globalInitializer; } - struct XmlReaderContext + struct XmlReader::XmlReaderContext { - xmlTextReaderPtr reader = nullptr; + using XmlTextReaderPtr = std::unique_ptr; + + XmlTextReaderPtr reader; bool readingAttributes = false; bool readingEmptyTag = false; + + explicit XmlReaderContext(XmlTextReaderPtr&& reader_) : reader(std::move(reader_)) {} }; XmlReader::XmlReader(const char* data, size_t length) @@ -416,38 +412,38 @@ namespace Azure { namespace Storage { namespace _internal { throw std::runtime_error("Xml data too big."); } - xmlTextReaderPtr reader - = xmlReaderForMemory(data, static_cast(length), nullptr, nullptr, 0); + auto reader = XmlReaderContext::XmlTextReaderPtr( + xmlReaderForMemory(data, static_cast(length), nullptr, nullptr, 0), xmlFreeTextReader); + if (!reader) { throw std::runtime_error("Failed to parse xml."); } - XmlReaderContext* context = new XmlReaderContext(); - context->reader = reader; - m_context = context; + m_context = std::make_unique(std::move(reader)); } - XmlReader::~XmlReader() + XmlReader::XmlReader(XmlReader&& other) noexcept { *this = std::move(other); } + + XmlReader& XmlReader::operator=(XmlReader&& other) noexcept { - if (m_context) - { - auto context = static_cast(m_context); - xmlFreeTextReader(static_cast(context->reader)); - delete context; - } + m_context = std::move(other.m_context); + return *this; } + XmlReader::~XmlReader() = default; + XmlNode XmlReader::Read() { - auto context = static_cast(m_context); + XmlReaderContext* context = m_context.get(); + xmlTextReader* reader = m_context->reader.get(); if (context->readingAttributes) { - int ret = xmlTextReaderMoveToNextAttribute(context->reader); + int ret = xmlTextReaderMoveToNextAttribute(reader); if (ret == 1) { - const char* name = reinterpret_cast(xmlTextReaderConstName(context->reader)); - const char* value = reinterpret_cast(xmlTextReaderConstValue(context->reader)); + const char* name = reinterpret_cast(xmlTextReaderConstName(reader)); + const char* value = reinterpret_cast(xmlTextReaderConstValue(reader)); return XmlNode{XmlNodeType::Attribute, name, value}; } else if (ret == 0) @@ -465,7 +461,7 @@ namespace Azure { namespace Storage { namespace _internal { return XmlNode{XmlNodeType::EndTag}; } - int ret = xmlTextReaderRead(context->reader); + int ret = xmlTextReaderRead(reader); if (ret == 0) { return XmlNode{XmlNodeType::End}; @@ -475,13 +471,13 @@ namespace Azure { namespace Storage { namespace _internal { throw std::runtime_error("Failed to parse xml."); } - int type = xmlTextReaderNodeType(context->reader); - bool is_empty = xmlTextReaderIsEmptyElement(context->reader) == 1; - bool has_value = xmlTextReaderHasValue(context->reader) == 1; - bool has_attributes = xmlTextReaderHasAttributes(context->reader) == 1; + int type = xmlTextReaderNodeType(reader); + bool is_empty = xmlTextReaderIsEmptyElement(reader) == 1; + bool has_value = xmlTextReaderHasValue(reader) == 1; + bool has_attributes = xmlTextReaderHasAttributes(reader) == 1; - const char* name = reinterpret_cast(xmlTextReaderConstName(context->reader)); - const char* value = reinterpret_cast(xmlTextReaderConstValue(context->reader)); + const char* name = reinterpret_cast(xmlTextReaderConstName(reader)); + const char* value = reinterpret_cast(xmlTextReaderConstValue(reader)); if (has_attributes) { @@ -520,49 +516,53 @@ namespace Azure { namespace Storage { namespace _internal { return Read(); } - struct XmlWriterContext + struct XmlWriter::XmlWriterContext { - xmlBufferPtr buffer; - xmlTextWriterPtr writer; + using XmlBufferPtr = std::unique_ptr; + using XmlTextWriterPtr = std::unique_ptr; + + XmlBufferPtr buffer; + XmlTextWriterPtr writer; + + explicit XmlWriterContext(XmlBufferPtr&& buffer_, XmlTextWriterPtr&& writer_) + : buffer(std::move(buffer_)), writer(std::move(writer_)) + { + } }; XmlWriter::XmlWriter() { XmlGlobalInitialize(); - auto buffer = xmlBufferCreate(); + auto buffer = XmlWriterContext::XmlBufferPtr(xmlBufferCreate(), xmlBufferFree); if (!buffer) { throw std::runtime_error("Failed to initialize xml writer."); } - auto writer = xmlNewTextWriterMemory(static_cast(buffer), 0); + auto writer = XmlWriterContext::XmlTextWriterPtr( + xmlNewTextWriterMemory(buffer.get(), 0), xmlFreeTextWriter); if (!writer) { - xmlBufferFree(static_cast(buffer)); throw std::runtime_error("Failed to initialize xml writer."); } - xmlTextWriterStartDocument(static_cast(writer), nullptr, nullptr, nullptr); + xmlTextWriterStartDocument(writer.get(), nullptr, nullptr, nullptr); - auto context = new XmlWriterContext; - context->buffer = buffer; - context->writer = writer; - m_context = context; + m_context = std::make_unique(std::move(buffer), std::move(writer)); } - XmlWriter::~XmlWriter() + XmlWriter::XmlWriter(XmlWriter&& other) noexcept { *this = std::move(other); } + + XmlWriter& XmlWriter::operator=(XmlWriter&& other) noexcept { - if (m_context) - { - auto context = static_cast(m_context); - xmlFreeTextWriter(static_cast(context->writer)); - xmlBufferFree(static_cast(context->buffer)); - delete context; - } + m_context = std::move(other.m_context); + return *this; } + XmlWriter::~XmlWriter() = default; + namespace { inline xmlChar* BadCast(const char* x) { @@ -572,8 +572,7 @@ namespace Azure { namespace Storage { namespace _internal { void XmlWriter::Write(XmlNode node) { - auto context = static_cast(m_context); - xmlTextWriterPtr writer = context->writer; + xmlTextWriter* writer = m_context->writer.get(); if (node.Type == XmlNodeType::StartTag) { if (!node.HasValue) @@ -611,9 +610,8 @@ namespace Azure { namespace Storage { namespace _internal { std::string XmlWriter::GetDocument() { - auto context = static_cast(m_context); - xmlBufferPtr buffer = context->buffer; - return std::string(reinterpret_cast(buffer->content), buffer->use); + return std::string( + reinterpret_cast(m_context->buffer->content), m_context->buffer->use); } #endif