[libxml++] libxmlpp::DomParser not safe in lists by value - probably not a v alue type



The following code fails, with a segment fault,
on LINUX, g++3.2.1, yada yada yada.

	using namespace std;
	#include <libxml++/libxml++.h>
	int
	main(int argc, char** argv)
	{
  		list<xmlpp::DomParser> lst;
  		lst.push_back( xmlpp::DomParser() );
  		lst.push_back( xmlpp::DomParser() );
	}

The pointer version works:

	using namespace std;
	#include <libxml++/libxml++.h>
	int
	main(int argc, char** argv)
	{
  		list<xmlpp::DomParser*> lst;
  		lst.push_back( new xmlpp::DomParser() );
  		lst.push_back( new xmlpp::DomParser() );
	}

This is almost 100% likely to occur because xmlpp::DomParser
is not a value type - it is not supposed to be copied as
a value.

(I'm reporting this bug before I try a fix, just in case
I get sidetracked into something else.)

Now, I'm not sure whether this is intentional or not.
But I cannot see any reason why it should be.
And I come from the school that says, if you don't
want something to be treated like a first class value object,
then remove (make private) the copy constructor and assignment 
methods.

Code inspection reveals that DomParser has a _doc pointer
member, which could be released twice after a copy.
There's no reference counter or the like on the pointed
to document, or it's _impl.

Anyway, that's enough code review for the initial bug report.
I'll go look for the proper fix, but I'd appreciate email
telling me whether xmlpp::DomParser should be treatable as a value,
with copy constructors, etc., or whether it should be handled
always via pointers.  That's a style question, and y'all own 
the library.

By the way, if you agree with me, that values are a good thing,
then the little test snippet above is something that can be applied
to any value type that has a default constructor.
Make it into a template

    template<typename T> test_push_value() {
  		list<T> lst;
  		lst.push_back( T() );
  		lst.push_back( T() );
    }

(and arrange for it to be called from your test suite).

this sort of thing is such a basic test
(along with its cousins, that test things like
basic constructors) 
and they catch a surprisingly large number of bugs.






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