[libxml++] Move some code from DtdValidator to Dtd



commit 293594ffd7bc810564d2868968487830a90559b6
Author: Kjell Ahlstedt <kjell ahlstedt bredband net>
Date:   Thu Oct 1 12:45:42 2015 +0200

    Move some code from DtdValidator to Dtd
    
    * libxml++/dtd.[h|cc]:
    * libxml++/validators/dtdvalidator.[h|cc]: Move the code for parsing a DTD
    from xmlpp::DtdValidator to xmlpp::Dtd. The code is arranged like that in
    the other validators. Bug #754673.

 libxml++/dtd.cc                     |  116 ++++++++++++++++++++++++++++++++---
 libxml++/dtd.h                      |  108 ++++++++++++++++++++++++++++----
 libxml++/validators/dtdvalidator.cc |  114 ++++++++++++++++------------------
 libxml++/validators/dtdvalidator.h  |   42 +++++++++++--
 4 files changed, 292 insertions(+), 88 deletions(-)
---
diff --git a/libxml++/dtd.cc b/libxml++/dtd.cc
index 13703a7..2ec0c15 100644
--- a/libxml++/dtd.cc
+++ b/libxml++/dtd.cc
@@ -5,45 +5,143 @@
  */
 
 #include <libxml++/dtd.h>
+#include <libxml++/exceptions/parse_error.h>
+#include <libxml++/io/istreamparserinputbuffer.h>
 
 #include <libxml/tree.h>
 
 namespace xmlpp
 {
   
-Dtd::Dtd(_xmlDtd* dtd)
-: impl_(dtd)
+struct Dtd::Impl
 {
-  dtd->_private = this;
+  Impl() : dtd(nullptr), is_dtd_owner(false) {}
+
+  _xmlDtd* dtd;
+  bool is_dtd_owner;
+};
+
+Dtd::Dtd()
+: pimpl_(new Impl)
+{
+}
+
+Dtd::Dtd(_xmlDtd* dtd, bool take_ownership)
+: pimpl_(new Impl)
+{
+  pimpl_->dtd = dtd;
+  if (dtd)
+  {
+    pimpl_->dtd->_private = this;
+    pimpl_->is_dtd_owner = take_ownership;
+  }
+}
+
+Dtd::Dtd(const std::string& filename)
+: pimpl_(new Impl)
+{
+  parse_subset("", filename);
+}
+
+Dtd::Dtd(const Glib::ustring& external, const Glib::ustring& system)
+: pimpl_(new Impl)
+{
+  parse_subset(external, system);
 }
 
 Dtd::~Dtd()
-{ 
+{
+  release_underlying();
+}
+
+void Dtd::parse_file(const std::string& filename)
+{
+  parse_subset("", filename);
+}
+
+void Dtd::parse_subset(const Glib::ustring& external, const Glib::ustring& system)
+{
+  release_underlying(); // Free any existing dtd.
+  xmlResetLastError();
+
+  auto dtd = xmlParseDTD(
+    external.empty() ? nullptr : (const xmlChar*)external.c_str(),
+    system.empty() ? nullptr : (const xmlChar*)system.c_str());
+
+  if (!dtd)
+  {
+    throw parse_error("Dtd could not be parsed.\n" + format_xml_error());
+  }
+
+  pimpl_->dtd = dtd;
+  pimpl_->dtd->_private = this;
+  pimpl_->is_dtd_owner = true;
+}
+
+void Dtd::parse_memory(const Glib::ustring& contents)
+{
+  // Prepare an istream with buffer
+  std::istringstream is(contents);
+
+  parse_stream(is);
+}
+
+void Dtd::parse_stream(std::istream& in)
+{
+  release_underlying(); // Free any existing dtd.
+  xmlResetLastError();
+
+  IStreamParserInputBuffer ibuff(in);
+
+  auto dtd = xmlIOParseDTD(0, ibuff.cobj(), XML_CHAR_ENCODING_UTF8);
+
+  if (!dtd)
+  {
+    throw parse_error("Dtd could not be parsed.\n" + format_xml_error());
+  }
+
+  pimpl_->dtd = dtd;
+  pimpl_->dtd->_private = this;
+  pimpl_->is_dtd_owner = true;
 }
 
 Glib::ustring Dtd::get_name() const
 {
-  return (char*)impl_->name;
+  return (pimpl_->dtd && pimpl_->dtd->name) ? (const char*)pimpl_->dtd->name : "";
 }
 
 Glib::ustring Dtd::get_external_id() const
 {
-  return (char*)impl_->ExternalID;
+  return (pimpl_->dtd && pimpl_->dtd->ExternalID) ? (const char*)pimpl_->dtd->ExternalID : "";
 }
 
 Glib::ustring Dtd::get_system_id() const
 {
-  return (char*)impl_->SystemID;
+  return (pimpl_->dtd && pimpl_->dtd->SystemID) ? (const char*)pimpl_->dtd->SystemID : "";
 }
 
 _xmlDtd* Dtd::cobj()
 {
-  return impl_;
+  return pimpl_->dtd;
 }
 
 const _xmlDtd* Dtd::cobj() const
 {
-  return impl_;
+  return pimpl_->dtd;
+}
+
+void Dtd::release_underlying()
+{
+  if (pimpl_->dtd)
+  {
+    pimpl_->dtd->_private = nullptr;
+    if (pimpl_->is_dtd_owner)
+    {
+      xmlFreeDtd(pimpl_->dtd);
+      pimpl_->is_dtd_owner = false;
+    }
+    pimpl_->dtd = nullptr;
+  }
 }
 
 } //namespace xmlpp
diff --git a/libxml++/dtd.h b/libxml++/dtd.h
index 6198072..30a6bfd 100644
--- a/libxml++/dtd.h
+++ b/libxml++/dtd.h
@@ -7,27 +7,107 @@
 #ifndef __LIBXMLPP_DTD_H
 #define __LIBXMLPP_DTD_H
 
-#include <libxml++/attribute.h>
-#include <list>
-#include <map>
+#include <libxml++/noncopyable.h>
+#include <glibmm/ustring.h>
+#include <string>
+#include <memory> // std::unique_ptr
 
 #ifndef DOXYGEN_SHOULD_SKIP_THIS
 extern "C" {
   struct _xmlDtd;
 }
-#endif //DOXYGEN_SHOULD_SKIP_THIS4
+#endif //DOXYGEN_SHOULD_SKIP_THIS
 
 namespace xmlpp
 {
 
-/** Represents XML DTDs.
- *
+//TODO: Derive from Node?
+/** Represents an XML DTD for validating XML files.
+ * DTD = Document Type Definition
  */
-class Dtd //TODO: Derive from Node?
+class Dtd : public NonCopyable
 {
 public:
-  Dtd(_xmlDtd* dtd);
-  ~Dtd();
+  Dtd();
+
+  /** Create a Dtd from the underlying libxml DTD element.
+   * @param dtd A pointer to the libxml DTD element.
+   * @param take_ownership If <tt>true</tt>, this Dtd instance takes ownership of
+   *        the libxml DTD element. The caller must not delete it.<br>
+   *        If <tt>false</tt>, this Dtd does not take ownership of the libxml
+   *        DTD element. The caller must guarantee that the libxml DTD element
+   *        exists as long as this Dtd keeps a pointer to it. The caller is
+   *        responsible for deleting the libxml DTD element when it's no longer
+   *        needed, unless it belongs to a Document, in which case it's deleted
+   *        when the Document is deleted.
+   */
+  explicit Dtd(_xmlDtd* dtd, bool take_ownership = false);
+
+  /** Create a Dtd and parse an external subset (DTD file) immediately.
+   *
+   * @newin{3,0}
+   *
+   * @param filename The URL of the DTD.
+   * @throws xmlpp::parse_error
+   */
+  explicit Dtd(const std::string& filename);
+
+  /** Create a Dtd and parse an external subset (DTD file) immediately.
+   *
+   * @newin{3,0}
+   *
+   * @param external The external ID of the DTD.
+   * @param system The URL of the DTD.
+   * @throws xmlpp::parse_error
+   */
+  Dtd(const Glib::ustring& external, const Glib::ustring& system);
+
+  ~Dtd() override;
+
+  /** Parse an external subset (DTD file).
+   * If another DTD has been parsed before, that DTD is replaced by the new one
+   * (deleted if this Dtd owns it).
+   *
+   * @newin{3,0}
+   *
+   * @param filename The URL of the DTD.
+   * @throws xmlpp::parse_error
+   */
+  void parse_file(const std::string& filename);
+
+  /** Parse an external subset (DTD file).
+   * If another DTD has been parsed before, that DTD is replaced by the new one
+   * (deleted if this Dtd owns it).
+   *
+   * @newin{3,0}
+   *
+   * @param external The external ID of the DTD.
+   * @param system The URL of the DTD.
+   * @throws xmlpp::parse_error
+   */
+  void parse_subset(const Glib::ustring& external, const Glib::ustring& system);
+
+  /** Parse a DTD from a string.
+   * If another DTD has been parsed before, that DTD is replaced by the new one
+   * (deleted if this Dtd owns it).
+   *
+   * @newin{3,0}
+   *
+   * @param contents The DTD as a string.
+   * @throws xmlpp::parse_error
+   */
+  void parse_memory(const Glib::ustring& contents);
+
+  /** Parse a DTD from a stream.
+   * If another DTD has been parsed before, that DTD is replaced by the new one
+   * (deleted if this Dtd owns it).
+   *
+   * @newin{3,0}
+   *
+   * @param in The stream.
+   * @throws xmlpp::parse_error
+   */
+  void parse_stream(std::istream& in);
 
   Glib::ustring get_name() const;
   Glib::ustring get_external_id() const;
@@ -40,13 +120,15 @@ public:
   /** Access the underlying libxml implementation.
    */
   const _xmlDtd* cobj() const;
+
+protected:
+  void release_underlying();
+
 private:
-  _xmlDtd* impl_;
+  struct Impl;
+  std::unique_ptr<Impl> pimpl_;
 };
 
 } // namespace xmlpp
 
 #endif //__LIBXMLPP_DTD_H
-
-
-
diff --git a/libxml++/validators/dtdvalidator.cc b/libxml++/validators/dtdvalidator.cc
index d91fb05..f5b718e 100644
--- a/libxml++/validators/dtdvalidator.cc
+++ b/libxml++/validators/dtdvalidator.cc
@@ -21,23 +21,39 @@
 namespace xmlpp
 {
 
+struct DtdValidator::Impl
+{
+  Impl() : dtd(nullptr), is_dtd_owner(false), context(nullptr) {}
+
+  Dtd* dtd;
+  bool is_dtd_owner;
+  _xmlValidCtxt* context;
+};
+
+
 DtdValidator::DtdValidator()
-: context_(nullptr), dtd_(nullptr)
+: pimpl_(new Impl)
 {
 }
 
 DtdValidator::DtdValidator(const std::string& filename)
-: context_(nullptr), dtd_(nullptr)
+: pimpl_(new Impl)
 {
-  parse_subset("", filename);
+  parse_file(filename);
 }
 
 DtdValidator::DtdValidator(const Glib::ustring& external, const Glib::ustring& system)
-: context_(nullptr), dtd_(nullptr)
+: pimpl_(new Impl)
 {
   parse_subset(external, system);
 }
 
+DtdValidator::DtdValidator(Dtd* dtd, bool take_ownership)
+: pimpl_(new Impl)
+{
+  set_dtd(dtd, take_ownership);
+}
+
 DtdValidator::~DtdValidator()
 {
   release_underlying();
@@ -45,86 +61,65 @@ DtdValidator::~DtdValidator()
 
 void DtdValidator::parse_file(const std::string& filename)
 {
-  parse_subset("", filename);
+  set_dtd(new Dtd(filename), true);
 }
 
 void DtdValidator::parse_subset(const Glib::ustring& external, const Glib::ustring& system)
 {
-  release_underlying(); // Free any existing dtd.
-  xmlResetLastError();
-
-  auto dtd = xmlParseDTD(
-    external.empty() ? 0 : (const xmlChar *)external.c_str(),
-    system.empty() ? 0 : (const xmlChar *)system.c_str());
-
-  if (!dtd)
-  {
-    throw parse_error("Dtd could not be parsed.\n" + format_xml_error());
-  }
-
-  Node::create_wrapper(reinterpret_cast<xmlNode*>(dtd));
-  dtd_ = static_cast<Dtd*>(dtd->_private);
+  set_dtd(new Dtd(external, system), true);
 }
 
 void DtdValidator::parse_memory(const Glib::ustring& contents)
 {
-  // Prepare an istream with buffer
-  std::istringstream is( contents );
-
-  parse_stream( is );
+  std::unique_ptr<Dtd> dtd(new Dtd());
+  dtd->parse_memory(contents);
+  set_dtd(dtd.release(), true);
 }
 
 void DtdValidator::parse_stream(std::istream& in)
 {
-  release_underlying(); // Free any existing dtd.
-  xmlResetLastError();
-
-  IStreamParserInputBuffer ibuff( in );
-
-  auto dtd = xmlIOParseDTD( 0, ibuff.cobj(), XML_CHAR_ENCODING_UTF8 );
-
-  if (!dtd)
-  {
-    throw parse_error("Dtd could not be parsed.\n" + format_xml_error());
-  }
+  std::unique_ptr<Dtd> dtd(new Dtd());
+  dtd->parse_stream(in);
+  set_dtd(dtd.release(), true);
+}
 
-  Node::create_wrapper(reinterpret_cast<xmlNode*>(dtd));
-  dtd_ = static_cast<Dtd*>(dtd->_private);
+void DtdValidator::set_dtd(Dtd* dtd, bool take_ownership)
+{
+  release_underlying();
+  pimpl_->dtd = dtd;
+  pimpl_->is_dtd_owner = take_ownership;
 }
 
 void DtdValidator::initialize_context()
 {
   Validator::initialize_context();
 
-  if (context_)
+  if (pimpl_->context)
   {
     //Tell the validation context about the callbacks:
-    context_->error = &callback_validity_error;
-    context_->warning = &callback_validity_warning;
+    pimpl_->context->error = &callback_validity_error;
+    pimpl_->context->warning = &callback_validity_warning;
 
     //Allow the callback_validity_*() methods to retrieve the C++ instance:
-    context_->userData = this;
+    pimpl_->context->userData = this;
   }
 }
 
 void DtdValidator::release_underlying()
 {
-  if (context_)
+  if (pimpl_->context)
   {
-    context_->userData = nullptr; //Not really necessary.
+    pimpl_->context->userData = nullptr; //Not really necessary.
 
-    xmlFreeValidCtxt(context_);
-    context_ = nullptr;
+    xmlFreeValidCtxt(pimpl_->context);
+    pimpl_->context = nullptr;
   }
 
-  if (dtd_)
+  if (pimpl_->dtd)
   {
-    //Make a local pointer to the underlying xmlDtd object as the wrapper is destroyed first.
-    //After free_wrappers is called dtd_ will be invalid (e.g. delete dtd_)
-    auto dtd = dtd_->cobj();
-    Node::free_wrappers(reinterpret_cast<xmlNode*>(dtd));
-    xmlFreeDtd(dtd);
-    dtd_ = nullptr;
+    if (pimpl_->is_dtd_owner)
+      delete pimpl_->dtd;
+    pimpl_->dtd = nullptr;
   }
 
   Validator::release_underlying();
@@ -132,17 +127,17 @@ void DtdValidator::release_underlying()
 
 DtdValidator::operator bool() const noexcept
 {
-  return dtd_ != nullptr;
+  return pimpl_->dtd && pimpl_->dtd->cobj();
 }
 
 Dtd* DtdValidator::get_dtd()
 {
-  return dtd_;
+  return pimpl_->dtd;
 }
 
 const Dtd* DtdValidator::get_dtd() const
 {
-  return dtd_;
+  return pimpl_->dtd;
 }
 
 void DtdValidator::validate(const Document* document)
@@ -152,16 +147,16 @@ void DtdValidator::validate(const Document* document)
     throw internal_error("Document pointer cannot be 0.");
   }
 
-  if (!dtd_)
+  if (!pimpl_->dtd)
   {
     throw internal_error("No DTD to use for validation.");
   }
 
   // A context is required at this stage only
-  if (!context_)
-    context_ = xmlNewValidCtxt();
+  if (!pimpl_->context)
+    pimpl_->context = xmlNewValidCtxt();
 
-  if(!context_)
+  if (!pimpl_->context)
   {
     throw internal_error("Couldn't create validation context");
   }
@@ -169,7 +164,8 @@ void DtdValidator::validate(const Document* document)
   xmlResetLastError();
   initialize_context();
 
-  const bool res = (bool)xmlValidateDtd( context_, (xmlDoc*)document->cobj(), dtd_->cobj() );
+  const bool res = (bool)xmlValidateDtd(pimpl_->context, (xmlDoc*)document->cobj(),
+                   pimpl_->dtd->cobj());
 
   if (!res)
   {
diff --git a/libxml++/validators/dtdvalidator.h b/libxml++/validators/dtdvalidator.h
index ac546a7..4513f8c 100644
--- a/libxml++/validators/dtdvalidator.h
+++ b/libxml++/validators/dtdvalidator.h
@@ -35,22 +35,36 @@ public:
    */
   explicit DtdValidator(const Glib::ustring& external, const Glib::ustring& system);
 
+  /** Create a validator.
+   *
+   * @newin{3,0}
+   *
+   * @param dtd A pointer to the DTD to use when validating XML documents.
+   * @param take_ownership If <tt>true</tt>, the validator takes ownership of
+   *        the DTD. The caller must not delete it.<br>
+   *        If <tt>false</tt>, the validator does not take ownership of the DTD.
+   *        The caller must guarantee that the DTD exists as long as the
+   *        validator keeps a pointer to it. The caller is responsible for
+   *        deleting the DTD when it's no longer needed.
+   */
+  explicit DtdValidator(Dtd* dtd, bool take_ownership);
+
   ~DtdValidator() override;
 
   /** Parse an external subset (DTD file).
    * If the validator already contains a DTD, that DTD is deleted.
-   * @param external The external ID of the DTD.
-   * @param system The URL of the DTD.
+   * @param filename The URL of the DTD.
    * @throws xmlpp::parse_error
    */
-  void parse_subset(const Glib::ustring& external, const Glib::ustring& system);
+  void parse_file(const std::string& filename) override;
 
   /** Parse an external subset (DTD file).
    * If the validator already contains a DTD, that DTD is deleted.
-   * @param filename The URL of the DTD.
+   * @param external The external ID of the DTD.
+   * @param system The URL of the DTD.
    * @throws xmlpp::parse_error
    */
-  void parse_file(const std::string& filename) override;
+  void parse_subset(const Glib::ustring& external, const Glib::ustring& system);
 
   /** Parse a DTD from a string.
    * If the validator already contains a DTD, that DTD is deleted.
@@ -66,6 +80,19 @@ public:
    */
   void parse_stream(std::istream& in);
 
+  /** Set a DTD.
+   * If the validator already contains a DTD, that DTD is released
+   * (deleted if the validator owns the DTD).
+   * @param dtd A pointer to the DTD to use when validating XML documents.
+   * @param take_ownership If <tt>true</tt>, the validator takes ownership of
+   *        the DTD. The caller must not delete it.<br>
+   *        If <tt>false</tt>, the validator does not take ownership of the DTD.
+   *        The caller must guarantee that the DTD exists as long as the
+   *        validator keeps a pointer to it. The caller is responsible for
+   *        deleting the DTD when it's no longer needed.
+   */
+  void set_dtd(Dtd* dtd, bool take_ownership);
+
   /** Test whether a DTD has been parsed.
    * For instance
    * @code
@@ -98,8 +125,9 @@ protected:
   void initialize_context() override;
   void release_underlying() override;
 
-  _xmlValidCtxt* context_;
-  Dtd* dtd_;
+private:
+  struct Impl;
+  std::unique_ptr<Impl> pimpl_;
 };
 
 } // namespace xmlpp


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]