Re: random commits to ORBit stable

[Sebastian Wilhelmi <wilhelmi ira uka de>]

Hi Michael,

> > 	Trying to track down a bug in ORBit-stable on Power PC was ( and is )
> > made extremely, tediously, soul wrackingly difficult by the lack of a
> > suitable ChangeLog entry.
> > 
> > 	Furthermore I don't recall seeing the broken patch discussed here - it
> > looks like it just 'magically' appeared in CVS. This is _really_
> > under-appreciated in a stable branch.
> >
> > 	Can someone explain _why_ we need this optimization, where it was
> > discussed, and why it was committed ?
> 
> The relevant mail is at
> 
> http://mail.gnome.org/archives/orbit-list/2001-March/msg00053.html
> 
> Afterwards Morten fixed the bug, I introduced with that patch (cough,
> cough). That's the above cited patch. 
> 
> The whole point of this patch is to not decide at IDL-compile-time,
> which memory has to be freed in the skels, but at run-time, just because
> at compile-time you can't decide it. It's now decided by looking at the
> 4 bytes just before the string to free. It is either FREE_MARKER, in
> which case the string gets freed, or it is the length of the current
> string (i.e. some other number), in which case the string lies in the
> GIOPRecvBuffer, and must not be freed.

Ok, only morons reply to themselves, so here we go:

Of course the whole thing is busted. The problem is, that I rely on
free_marker being directly in front of the allocated mem to match the
position of the stringlen in GIOP, but with a union of double and
unsigned long the magic marker was not in directly in front of the mem
block, but (on i386) 4 bytes before. 

So case games with structure memers of ORBit_mem_info won't work, as
AFAICT you can't instruct C to place a member in the physical end of a
struct. So I added some obscure macros to always get the right position.
That should fix the problem. It might help you too. Actually in real
life the difference shouldn't be too big, because you won't find
0xfefefefe too often in GIOP messages as the last word in a sequence.
But of course it's plain wrong.
 
Bye,
Sebastian
-- 
Sebastian Wilhelmi
mailto:wilhelmi@ira.uka.de
http://goethe.ira.uka.de/~wilhelmi

Index: src/orb/allocators.c
===================================================================
RCS file: /cvs/gnome/ORBit/src/orb/allocators.c,v
retrieving revision 1.34.4.7
diff -p -u -b -B -r1.34.4.7 allocators.c
--- src/orb/allocators.c	2001/10/31 10:23:18	1.34.4.7
+++ src/orb/allocators.c	2002/02/07 13:15:50
@@ -12,6 +12,8 @@ x, ORBIT_ROOT_OBJECT(x)->refs); CORBA_Ob
 #endif
 
 #define FREE_MARKER_IS_ALLOCATED 0xfefefefe
+#define PTR_TO_FREE_MARKER(x) (*(((CORBA_unsigned_long *)(x)) - 1))
+#define MEMINFO_TO_FREE_MARKER(x) (PTR_TO_FREE_MARKER (MEMINFO_TO_PTR (x)))
 
 /* The memory chunk stuff */
 
@@ -84,7 +86,7 @@ ORBit_alloc_2(size_t block_size,
 #endif
 	block->free = freefunc;
 	block->func_data = func_data;
-	block->u.free_marker= FREE_MARKER_IS_ALLOCATED;
+	MEMINFO_TO_FREE_MARKER (block) = FREE_MARKER_IS_ALLOCATED;
 
 	return MEMINFO_TO_PTR(block);
 }
@@ -111,13 +113,14 @@ ORBit_free(gpointer mem, CORBA_boolean i
 	if (!mem)
 		return;
 
-	block = PTR_TO_MEMINFO (mem);
-
 	/* This block has not been allocated by CORBA_alloc. Instead
          * it is memory in the receive buffer, that has been used in
          * demarshalling. Let's just return. */
-	if (block->u.free_marker != FREE_MARKER_IS_ALLOCATED)
+	if (PTR_TO_FREE_MARKER (mem) != FREE_MARKER_IS_ALLOCATED)
 		return;
+
+	block = PTR_TO_MEMINFO (mem);
+
 
 #ifdef ORBIT_DEBUG
 	g_assert (block->magic == 0xdeadbeef);
Index: src/orb/allocators.h
===================================================================
RCS file: /cvs/gnome/ORBit/src/orb/allocators.h,v
retrieving revision 1.14.4.3
diff -p -u -b -B -r1.14.4.3 allocators.h
--- src/orb/allocators.h	2001/10/01 14:21:38	1.14.4.3
+++ src/orb/allocators.h	2002/02/07 13:15:50
@@ -40,24 +40,25 @@ typedef struct {
 	   the memory block itself */
 	ORBit_free_childvals free; /* function pointer to free function */
 	gpointer func_data;
-	/* This free_marker is used to distinguish between memory,
-	 * which has to be freed (in that case free_marker is
-	 * FREE_MARKER_IS_ALLOCATED [0xfefefefe]) or memory, which
-	 * must not be freed as it is a string residing in the receive
-	 * buffer of a request (in that case free_marker per GIOP is
-	 * the length of the following string, thus some number
-	 * guaranteed to be smaller than 0xfffffefe, I don't think, we
-	 * ever support transferring strings with a length of 4GB ;-)
+
+	/* The following double serves two purposes. Firstly it
+	 * ensures, that ORBit_mem_info's size is a multiple of 8
+	 * (which is a must). Secondly it leaves room for a special
+	 * marker used to distinguish between memory, which has to be
+	 * freed (in that case the marker is FREE_MARKER_IS_ALLOCATED
+	 * [0xfefefefe]) or memory, which must not be freed as it is a
+	 * string residing in the receive buffer of a request (in that
+	 * case the marker per GIOP is the length of the following
+	 * string, thus some number guaranteed to be smaller than
+	 * 0xfefefefe, I don't think, we ever support transferring
+	 * strings with a length of 4GB ;-)
 	 *
 	 * The marker is not 0xffffffff, because that is also -1.
 	 * Remember that the length in the receive buffer can have
 	 * swapped endianess, so using 0xffff0000 is a plan to
 	 * disaster.  
 	 */
-	union {
-		CORBA_unsigned_long free_marker;
-	        double align_me_properly;
-	} u;
+	gdouble align_me_properly_and_provide_space_for_marker;
 } ORBit_mem_info;
 
 gpointer ORBit_alloc(size_t block_size,