Re: [lmi] [PATCH] validate documents against schema when loading

[Greg Chicares]

On 2013-05-15 15:43Z, Vaclav Slavik wrote:
> 
> On 2013-04-30 15:37, Greg Chicares wrote:
> 
>> /opt/lmi/src/lmi/xml_lmi.cpp: In constructor 
>> `xml_lmi::dom_parser::dom_parser(const std::string&)':
>> /opt/lmi/src/lmi/xml_lmi.cpp:79: warning: `get_error_message' is deprecated 
>> (declared at /opt/lmi/third_party/include/xmlwrapp/tree_parser.h:159)
[...]
> If you want to collect all errors and warnings, then replacing with
> messages().print() does the trick:
> 
> diff --git a/xml_lmi.cpp b/xml_lmi.cpp
> index 041b3da..e4b1a38 100644
> --- a/xml_lmi.cpp
> +++ b/xml_lmi.cpp
> @@ -76,7 +76,7 @@ xml_lmi::dom_parser::dom_parser(std::string const& filename)
>          if(true == parser_->operator!())
>              {
>              throw std::runtime_error
> -                ("Parser failed: " + parser_->get_error_message()
> +                ("Parser failed: " + parser_->messages().print()
>                  );
>              }
>          }
> @@ -110,7 +110,7 @@ xml_lmi::dom_parser::dom_parser(char const* data, 
> std::size_t length)
>          if(true == parser_->operator!())
>              {
>              throw std::runtime_error
> -                ("Parser failed: " + parser_->get_error_message()
> +                ("Parser failed: " + parser_->messages().print()
>                  );
>              }
>          }
> @@ -156,7 +156,7 @@ xml_lmi::dom_parser::dom_parser(std::istream const& is)
>          if(true == parser_->operator!())
>              {
>              throw std::runtime_error
> -                ("Parser failed: " + parser_->get_error_message()
> +                ("Parser failed: " + parser_->messages().print()
>                  );
>              }
>          }

Applied 20131121T0002Z, revision 5843.

> In fact, that confuses me about this code a bit: throw_on_error is the
> default (and was before 0.7, in the form of allow_exceptions=true
> argument). So as far as I can tell, the affected code in
> xml_lmi::dom_parser::dom_parser() is never called:
> 
>         if(true == parser_->operator!())
>             {
>             throw std::runtime_error
>                 ("Parser failed: " + parser_->messages().print()
>                 );
>             }
> 
> Am I missing something?

You are correct. Furthermore, the following throw-expression is
also unreachable:

// context from elsewhere:
        typedef xml::tree_parser DomParser;
        boost::scoped_ptr<DomParser> parser_;

// overzealous checking:
        parser_.reset(new DomParser(data, length));
        if(0 == parser_.get())
            {
            throw std::runtime_error("Parser not initialized.");
            }

The operator new can throw, and the parser ctor can throw; but if they
don't, then the scoped_ptr can't be null (and its reset() and get()
are documented never to throw). I'll simplify this soon.

(These tests may be left over from an improvident attempt to use libxml++:
http://lists.gnu.org/archive/html/lmi/2006-10/msg00029.html
| Yes, it is only because libxmlpp library does not guarantee that after
| a successfull (exception-less) call to parse_file its operator bool()
| will return true. In other words, even if parse_file succeeds we can't
| be certain that parser has successfuly parsed the file.
But there's no reason to keep these useless checks seven years later.)

>> /opt/lmi/src/lmi/xml_lmi.cpp:238: warning: `__comp_ctor' is deprecated 
>> (declared at /opt/lmi/third_party/include/xmlwrapp/document.h:103)
>> /opt/lmi/src/lmi/workhorse.make:762: recipe for target `xml_lmi.o' failed
> 
> This just needs explicit creation of xml::node (to avoid confusion with using 
> filename argument elsewhere and to make it possible to eventually have 
> xml:document(filename) in the far future):
> 
> diff --git a/xml_lmi.cpp b/xml_lmi.cpp
> index 041b3da..e4b1a38 100644
> --- a/xml_lmi.cpp
> +++ b/xml_lmi.cpp
> @@ -235,7 +235,7 @@ xml::element const& xml_lmi::dom_parser::root_node
>  }
>  
>  xml_lmi::xml_document::xml_document(std::string const& root_node_name)
> -    :document_(new xml_lmi::Document(root_node_name.c_str()))
> +    :document_(new xml_lmi::Document(xml::element(root_node_name.c_str())))
>  {
>  }

Applied 20131121T0004Z, revision 5844.

I'll reply to the rest of your email soon. First, though, I'm
going to upgrade to xmlwrapp-0.7.1 .