Ronald,

I don't particularly like the idea of overloading the alignment define to serve a completely different purpose. Yes, 0 is an illegal value for this define, (it would presumably result in a divide by zero as we take ptrSize % alignment). But that doesn't mean it's a good idea to overload the illegal value for this define in order to imply something completely different about how XMemory should function.

So: if we need to alter the way XMemory works, we should come up with a separate mechanism for doing that.

But: You haven't convinced me that any of this is necessary.

I was the person who initially brought up the issue of the pointer alignment, and I'm still somewhat bothered by the need to tack on the block header. But I'm not sure we should further complicate this code with yet another option...when it hasn't been shown that there is any reason to do so.

Do you have any compelling reason why we need this additional option? An answer that "MSVC uses 16 byte alignment" is not sufficient for me: my answer would simply be "then we've got a bug in the MSVC port, don't we? (where we might presumably need to set XML_PLATFORM_NEW_BLOCK_ALIGNMENT to 16)".

You brought up this issue because you'd seen some instability in your code. Have you shown that:

(a) This issue is due to the block header or pointer alignment?
(b) This issue can't be fixed by setting XML_PLATFORM_NEW_BLOCK_ALIGNMENT?


James.


On Oct 23, 2003, at 2:03 AM, Ronald Landheer-Cieslak wrote:


ping!

This patch has been on the list for a couple of days now - comments?

It's pretty straight-forward, actually: if the platform new block alignment
is 0, the global manager (or the one passed to the delete operator, whichever
best applies) is always used for deleting - which makes sense because there
is no way to store the address of the memory manager anymore.


As it is possible to set this to 0, and as that would result in a serious bug
otherwise, I think this is something that needs committing :)


rlc

On Mon, Oct 20, 2003 at 02:09:39PM +0200, Ronald Landheer-Cieslak wrote:
I have attached the patch described below:
On Mon, Oct 20, 2003 at 01:38:55PM +0200, Ronald Landheer-Cieslak wrote:
On Fri, Oct 17, 2003 at 10:11:48AM -0700, [EMAIL PROTECTED] wrote:
You can already get compile-time alignment through a #define, which you
could also define to be 0, which would give you half of "b".
Please don't recommend something that will give me unspecified results:
the delete operator reads at p-(headersize) to get the pointer to the
memory manager. If (headersize == 0) it will read at p, which will contain
user data, which may be anything.


If you want, I can supply a patch that will make Xerces use the default
memory manager (always) if (headersize == 0) (or use the passed-in manager
in the case of the second delete operator).
HTH

rlc

-- Innocence ends when one is stripped of the delusion that one likes oneself.
-- Joan Didion, "On Self Respect"

Index: XMemory.cpp
===================================================================
RCS file: /home/cvspublic/xml-xerces/c/src/xercesc/util/XMemory.cpp,v
retrieving revision 1.8
diff -b -w -u -p -r1.8 XMemory.cpp
--- XMemory.cpp 25 Aug 2003 16:12:55 -0000 1.8
+++ XMemory.cpp 20 Oct 2003 11:28:18 -0000
@@ -113,6 +113,9 @@ void XMemory::operator delete(void* p)
{
if (p != 0)
{
+#if defined(XML_PLATFORM_NEW_BLOCK_ALIGNMENT) && !XML_PLATFORM_NEW_BLOCK_ALIGNMENT
+ XMLPlatformUtils::fgMemoryManager->deallocate(p);
+#else
size_t headerSize = XMLPlatformUtils::alignPointerForNewBlockAllocation(
sizeof(MemoryManager*));
void* const block = (char*)p - headerSize;
@@ -120,6 +123,7 @@ void XMemory::operator delete(void* p)
MemoryManager* const manager = *(MemoryManager**)block;
assert(manager != 0);
manager->deallocate(block);
+#endif
}
}


@@ -132,6 +136,9 @@ void XMemory::operator delete(void* p, M

if (p != 0)
{
+#if defined(XML_PLATFORM_NEW_BLOCK_ALIGNMENT) && !XML_PLATFORM_NEW_BLOCK_ALIGNMENT
+ manager->deallocate(p);
+#else
size_t headerSize = XMLPlatformUtils:: alignPointerForNewBlockAllocation(sizeof(MemoryManager*));
void* const block = (char*)p - headerSize;
assert(*(MemoryManager**)block == manager);
@@ -142,6 +149,7 @@ void XMemory::operator delete(void* p, M
*/
MemoryManager* pM = *(MemoryManager**)block;
pM->deallocate(block);
+#endif
}
}




---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

-- The sight of death frightens them [Earthers]. -- Kras the Klingon, "Friday's Child", stardate 3497.2

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to