Roberto Rigamonti wrote:
Hi, I'm having some troubles with XML serializing. I'm working on a
university project that consists of a 3D visor that should receive its
input via XML files, validate them using a set of XSD files (I've
...
Instead of pasting here snippets of code, I've uploaded the entire
project so everything can be compiled and tested without much effort, it
can be downloaded from here:
http://www.roby1984.netsons.org/3DVisor-0.0.1.tar.bz2
I suggest to compile it using the following procedure:
$ configure --prefix=/(path/to/dir)/sandbox
$ make
$ make install
to avoid filling your system with buggy executables ;-)
The files involved in managing XML are XMLParser.cc, XMLParser.hh and
XMLDOMErrorReporter.hh, the program seems to ignore the target_->flush()
command at line 444 in XMLParser.cc (XMLParser::serializeXML()) and
leave an empty file until the next routine invoked by main(), that is,
addSchema(), loads the file and finds it empty.
I'm going mad on those errors, it's about two weeks I'm trying to
resolve this bug...
The first thing I noted with your code is that it's full of memory leaks
of this variety:
XMLParser.cc: DOMElement* addedElem =
xmlDoc->createElement(XMLString::transcode("xsd:element"));
The pointer returned by XMLString::transcode() must be freed through a
call to XMLString::release(). This is clear if you read the
documentation for XMLString::transcode().
Also, I would suggest that rather than transcoding these hard-coded
strings at run-time, that you create versions of these strings that are
already encoded in UTF-16 at compile time. You can use the constants
defined in xerces/util/XMLUniDefs.hpp. For an example of what I mean,
take a look at XMLUni.cpp in the same directory.
The reason the file is empty is because you never destroy the
LocalFileFormatTarget, so the underlying file is not closed until the
process exits. This is also a memory leak, and you have a memory leak
when you construct it, because you call XMLString::transcode() and you
don't release the pointer that's returned:
target_ = new LocalFileFormatTarget(XMLString::transcode(xmlFile.c_str()));
You can make your life easier by creating the LocalFileFormatTarget on
the stack, instead of on the heap. It's a small object, so there's no
danger in overflowing the stack.
Note also that LocalFileFormatTarget has a constructor that takes a
string in the local code page, so there's no need to do the transcoding
yourself:
LocalFileFormatTarget target_(xmlFile.c_str());
A few other comments, while I'm on a roll:
void
XMLParser::destroyInstance()
{
if(instance_)
delete(instance_);
instance_ = 0;
}
It's never necessary in C++ to check a pointer for null before deleting
it. The compiler already has to do this, so why have your code do it too?
if(current->getNodeType() != 0 && current->getNodeType() ==
DOMNode::ELEMENT_NODE)
I'm not sure why you're checking for a node type that's not equal to 0
then checking for one that's equal to DOMNode::ELEMENT_NODE.
getNodeType() return an enum, and an enum can only have a single value,
so the only test you need is for DOMNode::ELEMENT_NODE.
DOMElement* element = dynamic_cast< DOMElement* >(current);
if(XMLString::equals(element->getTagName(),
XMLString::transcode(tagName.c_str()))) {
The dynamic_cast here is unnecessary, and wastes cycles. You've already
checked that it's a DOMElement node. Also, the call to
XMLString::transcode() is a memory leak.
Dave
