Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XML: Use RAII wrappers instead of manual memory management #5767

Merged
merged 11 commits into from
Jul 30, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -37,44 +37,39 @@ namespace Azure { namespace Storage { namespace _internal {

class XmlReader final {
rschu1ze marked this conversation as resolved.
Show resolved Hide resolved
public:
explicit XmlReader(const char* data, size_t length);
rschu1ze marked this conversation as resolved.
Show resolved Hide resolved
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;
rschu1ze marked this conversation as resolved.
Show resolved Hide resolved
~XmlReader();

XmlNode Read();

private:
void* m_context = nullptr;
struct XmlReaderContext;
std::unique_ptr<XmlReaderContext> m_context;
};

class XmlWriter final {
public:
explicit XmlWriter();
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);

std::string GetDocument();

private:
void* m_context = nullptr;
struct XmlWriterContext;
std::unique_ptr<XmlWriterContext> m_context;
};

void XmlGlobalInitialize();
void XmlGlobalDeinitialize();

}}} // namespace Azure::Storage::_internal
123 changes: 69 additions & 54 deletions sdk/storage/azure-storage-common/src/xml_wrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <limits>
#include <memory>
#include <stdexcept>
#include <mutex>

#if defined(AZ_PLATFORM_WINDOWS)
#if !defined(WIN32_LEAN_AND_MEAN)
Expand All @@ -27,6 +28,9 @@ namespace Azure { namespace Storage { namespace _internal {

#if defined(AZ_PLATFORM_WINDOWS)

void XmlGlobalInitialize() {}
void XmlGlobalDeinitialize() {}

struct XmlReaderContext
{
XmlReaderContext()
Expand Down Expand Up @@ -392,19 +396,29 @@ namespace Azure { namespace Storage { namespace _internal {

#else

struct XmlGlobalInitializer final
void XmlGlobalInitialize()
{
XmlGlobalInitializer() { xmlInitParser(); }
~XmlGlobalInitializer() { xmlCleanupParser(); }
};
static std::once_flag flag;
std::call_once(flag, [] { xmlInitParser(); });
}

static void XmlGlobalInitialize() { static XmlGlobalInitializer globalInitializer; }
void XmlGlobalDeinitialize()
{
static std::once_flag flag;
std::call_once(flag, [] { xmlCleanupParser(); });
Jinming-Hu marked this conversation as resolved.
Show resolved Hide resolved
}

struct XmlReaderContext
struct XmlReader::XmlReaderContext
{
xmlTextReaderPtr reader = nullptr;
using XmlTextReaderPtr = std::unique_ptr<xmlTextReader, decltype(&xmlFreeTextReader)>;
Jinming-Hu marked this conversation as resolved.
Show resolved Hide resolved

XmlTextReaderPtr reader;
bool readingAttributes = false;
bool readingEmptyTag = false;

explicit XmlReaderContext(XmlTextReaderPtr && reader_)
: reader(std::move(reader_))
{}
};

XmlReader::XmlReader(const char* data, size_t length)
Expand All @@ -416,38 +430,38 @@ namespace Azure { namespace Storage { namespace _internal {
throw std::runtime_error("Xml data too big.");
}

xmlTextReaderPtr reader
= xmlReaderForMemory(data, static_cast<int>(length), nullptr, nullptr, 0);
auto reader
= XmlReaderContext::XmlTextReaderPtr(xmlReaderForMemory(data, static_cast<int>(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<XmlReaderContext>(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<XmlReaderContext*>(m_context);
xmlFreeTextReader(static_cast<xmlTextReaderPtr>(context->reader));
delete context;
}
m_context = std::move(other.m_context);
return *this;
}

XmlReader::~XmlReader() = default;

XmlNode XmlReader::Read()
{
auto context = static_cast<XmlReaderContext*>(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<const char*>(xmlTextReaderConstName(context->reader));
const char* value = reinterpret_cast<const char*>(xmlTextReaderConstValue(context->reader));
const char* name = reinterpret_cast<const char*>(xmlTextReaderConstName(reader));
const char* value = reinterpret_cast<const char*>(xmlTextReaderConstValue(reader));
return XmlNode{XmlNodeType::Attribute, name, value};
}
else if (ret == 0)
Expand All @@ -465,7 +479,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};
Expand All @@ -475,13 +489,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<const char*>(xmlTextReaderConstName(context->reader));
const char* value = reinterpret_cast<const char*>(xmlTextReaderConstValue(context->reader));
const char* name = reinterpret_cast<const char*>(xmlTextReaderConstName(reader));
const char* value = reinterpret_cast<const char*>(xmlTextReaderConstValue(reader));

if (has_attributes)
{
Expand Down Expand Up @@ -520,49 +534,53 @@ namespace Azure { namespace Storage { namespace _internal {
return Read();
}

struct XmlWriterContext
struct XmlWriter::XmlWriterContext
{
xmlBufferPtr buffer;
xmlTextWriterPtr writer;
using XmlBufferPtr = std::unique_ptr<xmlBuffer, decltype(&xmlBufferFree)>;
using XmlTextWriterPtr = std::unique_ptr<xmlTextWriter, decltype(&xmlFreeTextWriter)>;

XmlBufferPtr buffer;
XmlTextWriterPtr writer;

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<xmlBufferPtr>(buffer), 0);
auto writer = XmlWriterContext::XmlTextWriterPtr(xmlNewTextWriterMemory(buffer.get(), 0), xmlFreeTextWriter);

if (!writer)
{
xmlBufferFree(static_cast<xmlBufferPtr>(buffer));
throw std::runtime_error("Failed to initialize xml writer.");
}

xmlTextWriterStartDocument(static_cast<xmlTextWriterPtr>(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<XmlWriterContext>(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<XmlWriterContext*>(m_context);
xmlFreeTextWriter(static_cast<xmlTextWriterPtr>(context->writer));
xmlBufferFree(static_cast<xmlBufferPtr>(context->buffer));
delete context;
}
m_context = std::move(other.m_context);
return *this;
}


XmlWriter::~XmlWriter() = default;

namespace {
inline xmlChar* BadCast(const char* x)
{
Expand All @@ -572,8 +590,7 @@ namespace Azure { namespace Storage { namespace _internal {

void XmlWriter::Write(XmlNode node)
{
auto context = static_cast<XmlWriterContext*>(m_context);
xmlTextWriterPtr writer = context->writer;
xmlTextWriter * writer = m_context->writer.get();
if (node.Type == XmlNodeType::StartTag)
{
if (!node.HasValue)
Expand Down Expand Up @@ -611,9 +628,7 @@ namespace Azure { namespace Storage { namespace _internal {

std::string XmlWriter::GetDocument()
{
auto context = static_cast<XmlWriterContext*>(m_context);
xmlBufferPtr buffer = context->buffer;
return std::string(reinterpret_cast<const char*>(buffer->content), buffer->use);
return std::string(reinterpret_cast<const char*>(m_context->buffer->content), m_context->buffer->use);
}

#endif
Expand Down
Loading