senjuhashirama / pugixml

Automatically exported from code.google.com/p/pugixml
0 stars 0 forks source link

xml_document.load_buffer() is not "const" on the input string. #156

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Dear Sirs, here's my experience (first experience I have to say) with Pugixml.
First of all I have to say it's very interesting and well done.

What steps will reproduce the problem?
Well... first of all I'm testing Pugixml because it let me use my own 
allocator/deallocator, a requirement for the project I'm developing.
Well... I have an allocated char buffer and I call the load_buffer() function 
to load a new xml.

Some sample code

char *myBuf = xw::md::alloc(size+1) ;
// feed mybuf with a null terminated string contaning the xml.
// The string has 'size' valid chars and a null.
pugi::xml_document doc ;
pugi::xml_parse_result r = doc.load_buffer(myBuf, size) ;

// Read the node named "rsp", look for it's attribute 'ack'.
// If '1' is the value of 'ack' than copy the content into the parent node
// and remove the 'rsp' node.
pugi::xml_node rsp = doc.child("rsp") ;
std::string ack(rsp.attribute("ack").value()) ;
if ( 1 == xw::str::strtol(ack.c_str()) ) { // as strtol() in C.
   // Cutting the rsp, taking only first child...
   xw::commander::pugiXmlStringWriter w ;
   doc.append_copy(rsp.first_child()) ;
   doc.remove_child("rsp") ;
   doc.save(w) ;
   // now, use the result I got in w.result.c_str()...
   // now free the original 'myBuf' buffer,
   // alloc it again for w.result.length()
   // and copy w.result.c_str() in it
}

... something mode: this is the writer sample code within mi namespace 
xw::commander

struct pugiXmlStringWriter: pugi::xml_writer
{
   xw::md::string_t result;

   virtual void write(const void* data, size_t size)
   {
      result.append((const char *)data, size) ;
   }
};

Ok... when I free 'myBuf' that's been used in load_buffer() after the 
pugi::xml_document has been changed and before to copy in it the result of the 
save() function, I have a memory corruption.

So what did I do to work it around?
I allocate another buf, call it mySecondBuf. Copy all data from myBuf in it and 
use doc.load_buffer_inplace() instead of load_buffer(), in order to save 
memory. Than, after the changes are done, I can free 'myBuf', allocate it 
again, save in it all data from the writer, and then free 'mySecondBuf' when 
I'm sure I'll never use doc anymore.

This way works with no memory effects. I think there must be something 
incorrect in load_buffer() method since it's behaviour isn't the one expected 
reading documentation.

Which version of pugixml are you using? On what operating system/compiler?
Verision 1.2, compiler Visual Studio 2008 SP1 on Windows 7 professional

Please provide any additional information below.

Original issue reported on code.google.com by roberto....@gmail.com on 9 May 2012 at 3:54

GoogleCodeExporter commented 9 years ago
load_buffer() does not hold to the pointer you're passing it - it copies the 
buffer contents to a newly-allocated buffer. The buffer also does not need to 
be null-terminated (although it does not hurt, of course). What you're seeing 
here is probably something else.

Please, provide:
1. Complete usage code - with calls to free, etc.
2. Since you seem to be setting custom allocation/deallocation functions, 
please provide code for them as well if possible.
3. The details about memory corruption - i.e. how you're detecting it (heap 
assertion/debug check?) and the callstack for when it happens.

Original comment by arseny.k...@gmail.com on 9 May 2012 at 4:03

GoogleCodeExporter commented 9 years ago
Dear Sirs,
I'll try to provide some code reproducing the error as a synthesis of my 
project that is too wide to be sent completelly (one executable, 3 
libraries, some dll) and send it to you to solve the problem. I suppose 
you're right, maybe I'm facing something else (ad I think I already find 
what, after I wrote to you...)

Thanks anyway.

Original comment by roberto....@gmail.com on 9 May 2012 at 4:38

GoogleCodeExporter commented 9 years ago
Closing for lack of activity. Feel free to comment here if new information is 
available, I'll reopen the issue then.

Original comment by arseny.k...@gmail.com on 6 Jul 2012 at 11:22