[Orbit-python-list] Re: Changes in marshalling code between 0.5.3 and 0.5.6?

[Owen T. <ot...@re...>]

Roland Mas <ma...@ec...> writes:

> Roland Mas (2001-01-26 23:02:51 +0100) :
>=20
> >   As a developer for ORBit-Python, a Python binding for ORBit, I'm
> > encountering a problem.  You could even call that a bug, but I'm not
> > casting the blame to anyone (yet >:-] ).
>=20
>   Well, after a bit of hacking, it appears that given a
> GIOPRecvBuffer=A0*buf, the attribute
> GIOP_MESSAGE_BUFFER(buf)->message_header.message_size is too small by
> twelve bytes.  "Coincidentally", twelve is sizeof(GIOPMessageHeader)
> too.
>=20
>   So, we patched ORBit-Python to add this sizeof() to the message_size
> when checking for availability of bytes in the buffer, but it still
> feels uncomfortable.  So I'd like a confirmation that what we did was
> The Right Thing, and not a dirty hack to fix a broken behaviour in
> ORBit.  Should this be a broken behaviour, please keep me informed
> when it is fixed, so that we can clean the hack.

Well, to shed a little more light on the issue, the root cause
of the problem was Elliot's change of Nov 9:

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
revision 1.117.4.3
date: 2000/11/09 20:14:05;  author: sopwith;  state: Exp;  lines: +10 -11

src/IIOP/giop-msg-buffer.c: Avoid a memory corruption problem on the Alpha =
by doing
some magic voodoo that makes the symptoms go away. No clue what the actual =
cause is.

diff -u -r1.117.4.2 -r1.117.4.4
--- giop-msg-buffer.c   2000/10/27 19:40:52     1.117.4.2
+++ giop-msg-buffer.c   2000/11/15 02:10:36     1.117.4.4
@@ -56,6 +56,8 @@
   GIOP_octet flags;
 } PACKED;
=20
+#include <stdlib.h>
+
 /* functions */
 static gint giop_recv_decode_message(GIOPRecvBuffer *buf);
 static gboolean num_on_list(GIOP_unsigned_long num,
@@ -754,13 +756,11 @@
   if (buffer =3D=3D NULL)
     return;
=20
-  if(buffer->message_body) {
-    buffer->message_body =3D ((guchar *)buffer->message_body)
-      - sizeof(GIOPMessageHeader);
-=20=20=20=20
-    g_free(buffer->message_body);
-    buffer->message_body =3D NULL;
-  }
+  if(buffer->message_body)
+    {
+      g_free(buffer->message_body);
+      buffer->message_body =3D NULL;
+    }
=20
   if(GIOP_MESSAGE_BUFFER(buffer)->connection->incoming_msg =3D=3D buffer)
     GIOP_MESSAGE_BUFFER(buffer)->connection->incoming_msg =3D NULL;
@@ -888,11 +888,10 @@
          goto errout;
        }
=20
-       retval->message_body =3D g_malloc(message_size+sizeof(GIOPMessageHe=
ader));
+       retval->message_body =3D g_malloc(message_size+sizeof(GIOPMessageHe=
ader)+4);
        /* XXX1 This is a lame hack to work with the fact that
           alignment is relative to the MessageHeader, not the RequestHeade=
r */
-       retval->message_body =3D ((guchar *)retval->message_body) + sizeof(=
GIOPMessageHeader);
-       retval->cur =3D retval->message_body;
+       retval->cur =3D retval->message_body + 12;
        retval->state =3D GIOP_MSG_READING_BODY;
        retval->left_to_read =3D message_size;
        break;
@@ -1131,7 +1130,7 @@
 if(!( (( ((guchar*)GIOP_RECV_BUFFER(buf)->cur) \
                        + (requested_increment) ) \
                       <=3D ( ((guchar *)GIOP_RECV_BUFFER(buf)->message_bod=
y) \
-                          + GIOP_MESSAGE_BUFFER(buf)->message_header.messa=
ge_size)) \
+                          + GIOP_MESSAGE_BUFFER(buf)->message_header.messa=
ge_size) + 12) \
                      && ( ( ((guchar*)GIOP_RECV_BUFFER(buf)->cur) \
                            + (requested_increment) ) \
                          >=3D ((guchar*)GIOP_RECV_BUFFER(buf)->cur) ))) go=
to out
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

This is indeed magic voodoo, and I don't think changing the interpretation
of retval->message_body was necessary -- the reason why I don't think
it is necessary, is that the only places that used it were the
places that broke: orbit-perl, orbit-python, and ORBit. Yes, ORBit is
currently broken. (See code below)

The two possibilities of things that might have actually been fixed here
were:

1) Allocating message_body bigger (though that could only have covered
   up bugs elsewhere.)

2) Not counting on sizeof(GIOPMessageHeader) =3D=3D 12. Though if that were=
n't
   the case, the breakage would not be subtle.

Though perhaps changing message_body back is worse than the just leaving it
as it is now.

=3D=3D=3D=3D=3D=3D=3D
void ORBit_decode_CORBA_TypeCode(CORBA_TypeCode* t, GIOPRecvBuffer* buf)
{
        CDR_Codec codec_d;
        CDR_Codec* codec =3D &codec_d;
        TCDecodeContext ctx;
        GSList* l;

        CDR_codec_init_static(codec);
        codec->buffer=3Dbuf->cur;
        codec->release_buffer=3DCORBA_FALSE;
        codec->readonly=3DCORBA_TRUE;
        codec->buf_len =3D                /* hope this is correct */
          ((guchar *)buf->message_body) +
          GIOP_MESSAGE_BUFFER(buf)->message_header.message_size
          - ((guchar *)buf->cur);
=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20
        codec->data_endian=3DGIOP_MESSAGE_BUFFER(buf)->message_header.flags=
 & 1;

        ctx.current_idx=3D0;
        ctx.prior_tcs=3DNULL;
        tc_dec(t, codec, &ctx);
        for(l=3Dctx.prior_tcs;l;l=3Dl->next)
                g_free(l->data);
        g_slist_free(ctx.prior_tcs);
        buf->cur =3D ((guchar *)buf->cur) + codec->rptr;
}
=3D=3D=3D=3D=3D=3D

That hope is now pretty forlorn.

Regards,
                                        Owen