falcong / pugixml

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

Provide allocated memory length to the custom deallocate routine #154

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What feature do you require in pugixml?
The size of the memory allocated in the custom allocate routine to be passed to 
the custom deallocate routine.

What do you need this feature for?
My project erases memory before freeing it and so I need to know the length.  I 
can't get it from the "void *" allocated memory itself.

If you already have a patch, please attach it to the issue.
I don't have a patch but here is my current method of providing this without 
being given it (trashMemory takes 2 parameters - the memory and its length) - 
note this code has not been extensively tested:

const char *eye_catcher = "*pugixml_memory*";
const size_t extralen = strlen(eye_catcher) + sizeof(size_t);

static void *custom_allocate(size_t len)
{
  // Get more so we can store the length as we can't determine length in deallocate
  char *ptr = new (std::nothrow) char[len + extralen];

  if (ptr == NULL)
    return NULL;

  // Put eyecatcher at the beginning - less likely to be overwritten!
  memcpy(ptr, eye_catcher, strlen(eye_catcher));
  // Then the length requestor asked for
  memcpy(ptr + strlen(eye_catcher), &len, sizeof(size_t));

  // Return ptr to what they wanted
  return (void *)(ptr + extralen);
}

static void custom_deallocate(void *ptr)
{
  // Can't check is ours if pointer is NULL
  if (ptr == NULL)
    return;

  char *cptr = static_cast<char *>(ptr) - extralen;

  // Verify we allocated it
  if (memcmp(cptr, eye_catcher, strlen(eye_catcher)) != 0) {
    // No - delete as is
    delete[] static_cast<char *>(ptr);
    return;
  }

  size_t len;

  // Get length of user data stored after eyecatcher
  memcpy(&len, cptr + strlen(eye_catcher), sizeof(size_t));

  // Trash all data
  trashMemory(cptr, len + extralen);

  // Free all of it
  delete[] cptr;
}

Original issue reported on code.google.com by dk.2...@gmail.com on 29 Feb 2012 at 7:45

GoogleCodeExporter commented 9 years ago
This is technically doable, since it seems that in all cases either the 
allocation length is already known, or it is very cheap to store it.

However, the amount of use cases for that is very low... If you want to do 
memory tracking or some special markers for verification purposes, you will 
need to store the allocation length - for example, like you're doing right now 
- but you will need to do this in almost all other cases (i.e. if you override 
new/delete or for most other APIs with allocation/deallocation callbacks).

It makes sense to expose the size if it's readily available for performance 
reasons - to make size binning constant time - Lua does that, for example. 
However, this use case does not apply to pugixml since most allocation sizes 
are beyond that of typical small-block allocators.

I'll consider this enhancement, for now you'll have to store the length 
yourself (pugixml does exactly that in the test suite - see 
http://code.google.com/p/pugixml/source/browse/trunk/tests/allocator.cpp#87) 

Original comment by arseny.k...@gmail.com on 6 Mar 2012 at 3:17

GoogleCodeExporter commented 9 years ago
We have decided that we don't *need* this for the particular XML file we use 
pugixml to parse, although we do use this approach for other data in the 
application.

So, although I still think that the deallocation routine should be passed the 
length of the memory previously allocated and is requested to be freed, I agree 
with the priority being set to low.

Thanks for a great product!

Original comment by dk.2...@gmail.com on 7 Mar 2012 at 7:02

GoogleCodeExporter commented 9 years ago
I know I agreed to the low priority nearly 18 months ago, is there any 
likelihood it will be coming soon?

Original comment by dk.2...@gmail.com on 2 Oct 2013 at 2:29

GoogleCodeExporter commented 9 years ago
Very unlikely to happen. There is no demand for this feature, and it's not 
trivial to implement (need to carefully track sizes for all allocations since 
they are not readily available everywhere, implement extra tracking in the test 
suite to verify correctness and provide transparent backwards compatibility for 
people who were using the original callbacks).

There's something that I want to mention again - I was not very clear in my 
original comment. How do you track sizes for regular heap allocations?

I'm used to systems where the *default* is a deallocation callback without a 
size - a prime example being C++ operator delete, that does not have a size 
argument - and a deallocation callback with a size being an exception, used 
mainly for optimization (i.e. small-block allocators are typically more 
efficient if they know the allocation size while deallocating).

Original comment by arseny.k...@gmail.com on 15 Jan 2014 at 6:17

GoogleCodeExporter commented 9 years ago

Original comment by arseny.k...@gmail.com on 27 Jan 2014 at 12:47