[omniORB] CORBA::Any::NP_marshalDataOnly assertion failure

[Peter Housel]

On Mon, 2007-11-26 at 16:18 +0000, Duncan Grisby wrote:
> On Tuesday 20 November, "Peter S. Housel" wrote:
> 
> [...]
> > omniORB: Assertion failed.  This indicates a bug in the application
> > using omniORB, or maybe in omniORB itself.
> > 
> > file: /home/housel/sourceforge/omniorb/omni/src/lib/omniORB/dynamic/any.cc
> >  line: 433
> >  info: kind == CORBA::tk_void || kind == CORBA::tk_null
> > terminate called after throwing an instance of 'omniORB::fatalException'
> > Abort (core dumped)
> > 
> > The code in NP_marshalDataOnly seems to be asserting that only tk_null
> > and tk_void should be without a pd_data or pd_mbuf field, but this seems
> > to also be the case for nil object references.
> 
> That's a silly bug. The code just before the assertion that's triggering
> should accept tk_objref to cope with nil object references. I've fixed
> it in CVS.

That doesn't quite do it, because operator=() and the copy constructor
don't deal properly with nil object references either.  Changing the
test code to:

#include <omniORB4/CORBA.h>

int main(int argc, char *argv[]) {
  CORBA::Any a;
  a <<= CORBA::Object::_nil();
  cdrMemoryStream s;
  a.NP_marshalDataOnly(s);

  CORBA::Any b(a);
  b.NP_marshalDataOnly(s);

  CORBA::Any c = a;
  c.NP_marshalDataOnly(s);

  return 0;
}

yields the following failure:

omniORB: Assertion failed.  This indicates a bug in the application
using omniORB, or maybe in omniORB itself.

file: /home/housel/sourceforge/omniorb/omni/src/lib/omniORB/dynamic/any.cc
 line: 434
 info: pd_marshal
terminate called after throwing an instance of 'omniORB::fatalException'
Abort (core dumped)

Would copying pd_marshal in the else { ... } cases of the copy
constructor operator=() be the right thing to do?  Alternatively, maybe
the second branch should be "else if (a.pd_marshal) {" rather than
"else if (a.pd_data) {".

Mightn't it be a good idea to have a "pd_duplicate" member so that the
pd_data optimization could be maintained across a copy?

Also, it looks like "pd_mbuf = 0;" within operator=() is redundant,
since the call to PR_cleardata() does this already.

-Peter-