From: <and...@am...> - 2003-05-02 04:34:14
|
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. |