[ http://issues.apache.org/jira/browse/XERCESC-1378?page=comments#action_60986 ] Gareth Reakes commented on XERCESC-1378: ----------------------------------------
Hey, Nikolay Ognyanov (JIRA) wrote: > [ > http://issues.apache.org/jira/browse/XERCESC-1378?page=comments#action_60981 ] > Nikolay Ognyanov commented on XERCESC-1378: > ------------------------------------------- > > I agree with the part about BinMemInputStream. > > I disagree however about the local copy in MemBufInputSource. I see no > creation of such > copy anywhere in MemBufInputSource.cpp. Apart > from the destructor fSrcBytes appears only > in the 2 constructors like this : Exactly, no copy is created. However, MembufInputSource offers the facility to delete the string you passed in via the adoptBuffer parameter. If you set it to true then it will take care of deleting it for you. That way, if you newed the string in the calling class you no longer have to care about it. > MemBufInputSource::MemBufInputSource( const XMLByte* const srcDocBytes > , const unsigned int byteCount > , const XMLCh* const bufId > , const bool adoptBuffer > , MemoryManager* const manager) : > InputSource(bufId, manager) > , fAdopted(adoptBuffer) > , fByteCount(byteCount) > , fCopyBufToStream(true) > , fSrcBytes(srcDocBytes) > { > } > > Since srcDocBytes and fSrcBytes are pointers this initialization creates > copy of the pointer (srcDocBytes) and not copy of the content it points to. > If this was nor true BTW then it would be wrong again because copy would be > created unconditionally regardlis of the value of fAdopted. > > Another way to easily verify how wrong things are here > is to search in MemBufInputSource.cpp for fAdopted. Then you will see that it > only appears in initialization and in the destruction conditional I quoted. > This can not be true because creation of a copy of the arg string is supposed > to depend on it. No, the creation of a copy is not dependent on the flag - just whether the class should delete it. Having a parameter like this can tidy up calling code and offer possible optimizations (you only have to create 1 copy of the string). You will find this used in a fair few places in xerces. > > BTW - I found out this issue out not by mere code > inspection but the hard way after debugging some > "magic" segfaults... I hate those :) I never say never, but I don't think this is the issue. The only way I can see this happening is if you call 1) Make a MemBufInputSource with adopt set to true 2) call MemBufInputSource::setCopyBufToStream(false) 3) call makeStream() 4) delete the MemBufInputSource 5) do something with the BinMemInoutStream returned from makeStream If you do that then it will break. Regards, Gareth > wrong string destruction in MemBufInputSource::~MemBufInputSource() > ------------------------------------------------------------------- > > Key: XERCESC-1378 > URL: http://issues.apache.org/jira/browse/XERCESC-1378 > Project: Xerces-C++ > Type: Bug > Components: SAX/SAX2 > Versions: 2.6.0 > Environment: Any > Reporter: Nikolay Ognyanov > > MemBufInputSource::~MemBufInputSource() > { > if (fAdopted) > delete [] (XMLByte*)fSrcBytes; > } > This seems terribly wrong. Constructor initializes fSrcBytes > with its argument srcDocBytes and this never changes, so the > original argument string is deleted here and not a copy of it. > Actual creation and destruction of a copy if fAdopted is set > is handled in BinMemInputStream which is instantiated in > MemBufInputSource::makeStream(). Therefore the code quoted > above should be removed from the destructor and it should do > nothing. > Regards > Nikolay -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa - If you want more information on JIRA, or have a bug to report see: http://www.atlassian.com/software/jira --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
