From: DanB on

Well I finally bought the new 2008 VS. I'm moving carefully over as I
have to keep a 7.1 build current. I'm moving my support first as I don't
have to keep it updated. I started with that hexml project.

When I get to this as a copy constructor:

XMLNODESET::XMLNODESET( XMLNODESET& inSet )
{
*this= inSet;
}

dec:
class HE_XML_EXT_CLASS XMLNODESET
{
private:
std::vector<TiXmlNode*> set;
....
};

The pointer to the vector is copied now where as in 7.1 it would do a
deep copy of the vector.

This can't be right, is this a known issue? And I'm not sure how to fix
it if it is my problem.

Thanks, Dan.


From: Giovanni Dicanio on
"DanB" <abc(a)some.net> ha scritto nel messaggio
news:x9Yhn.5035$mn6.401(a)newsfe07.iad...

> When I get to this as a copy constructor:
>
> XMLNODESET::XMLNODESET( XMLNODESET& inSet )

I think proper copy ctor signature should be:

XMLNODESET::XMLNODESET( const XMLNODESET & )

Giovanni


From: David Webber on


"DanB" <abc(a)some.net> wrote in message
news:x9Yhn.5035$mn6.401(a)newsfe07.iad...

> Well I finally bought the new 2008 VS. I'm moving carefully over as I have
> to keep a 7.1 build current. I'm moving my support first as I don't have
> to keep it updated. I started with that hexml project.
>
> When I get to this as a copy constructor:
>
> XMLNODESET::XMLNODESET( XMLNODESET& inSet )
> {
> *this= inSet;
> }
>
> dec:
> class HE_XML_EXT_CLASS XMLNODESET
> {
> private:
> std::vector<TiXmlNode*> set;
> ...
> };
>
> The pointer to the vector is copied now where as in 7.1 it would do a deep
> copy of the vector.
>
> This can't be right, is this a known issue? And I'm not sure how to fix it
> if it is my problem.

I haven't had any problems. But I'd have written it as

XMLNODESET::XMLNODESET( const XMLNODESET & inSet )
: set( inSet.set )
{
}

with a *const* reference as an argument, and using the vector's copy
constructor.

Without seeing how you've defined the assignment operator, it's not possible
to see what your code is doing.

Dave

--
David Webber
Mozart Music Software
http://www.mozart.co.uk
For discussion and support see
http://www.mozart.co.uk/mozartists/mailinglist.htm

From: Giovanni Dicanio on
"DanB" <abc(a)some.net> ha scritto nel messaggio
news:x9Yhn.5035$mn6.401(a)newsfe07.iad...

> class HE_XML_EXT_CLASS XMLNODESET
> {
> private:
> std::vector<TiXmlNode*> set;
> ...
> };
>
> The pointer to the vector is copied now where as in 7.1 it would do a deep
> copy of the vector.

I'm not sure that deep-copying the above 'set' data member is a good robust
thing, because the pointers stored in the vector are copied.
What is the semantics of 'set'? Does the XMLNODESET class "own" the pointed
TiXmlNode's?
In this case, I would prefer using something like:

vector< shared_ptr< TiXmlNode > > set;

Giovanni


From: DanB on
Giovanni Dicanio wrote:
> "DanB" <abc(a)some.net> ha scritto nel messaggio
> news:x9Yhn.5035$mn6.401(a)newsfe07.iad...
>
>> class HE_XML_EXT_CLASS XMLNODESET
>> {
>> private:
>> std::vector<TiXmlNode*> set;
>> ...
>> };
>>
>> The pointer to the vector is copied now where as in 7.1 it would do a
>> deep copy of the vector.
>
> I'm not sure that deep-copying the above 'set' data member is a good
> robust thing, because the pointers stored in the vector are copied.
> What is the semantics of 'set'? Does the XMLNODESET class "own" the
> pointed TiXmlNode's?
> In this case, I would prefer using something like:
>
> vector< shared_ptr< TiXmlNode > > set;
>
> Giovanni
>
>
Thank you Giovanni, David.

The const qualifier did no fix this.

Yes, the copy is not deep, only the pointer set needs to be copied, not
what they point at.

The copy constructor itself is implicit. I'm returning the object from a
function call, like:

XMLNODESET CXML::GetNodeSet( LPCTSTR xpattern )
{
XMLNODESET set( this );

//note that 'this' in this case is the CXML object and why there is no
public constructor for XMLNODESET. You can only get XMLNODESETs from CXML.

if( xpattern == NULL )
return set;

CXPath xparse( documentNode->RootElement( ), currentNode, xpattern );
lastXPathError= xparse.GetError( );
lastXPathCount= xparse.GetCount( );

for( unsigned long i= 0; i < xparse.GetCount( ); ++i )
set.set.push_back( xparse.GetNode( i ) );

return set;
}

XMLNODESET is little used and small so didn't see a reason to return a
pointer to one of them. Also, this call is in a dll and in the app
called like:

nodeset= xml.GetNodeSet( "//*/subStuff/testing[@type=1]" );

But like I say, it works fine in 7.1. I get a fresh separate copy of the
vector.

Thanks, Dan.