Mike Hearn wrote:
This corrects an attempt to demarshal garbage during InstallShield10 startup. For full InstallShield 10 support we need a stdole2.tlb
Mike Hearn mh@codeweavers.com Correctly marshal NULL ppUnk refptrs in NdrPointerMarshall
Index: dlls/rpcrt4/ndr_marshall.c
RCS file: /home/wine/wine/dlls/rpcrt4/ndr_marshall.c,v retrieving revision 1.24 diff -u -p -d -u -r1.24 ndr_marshall.c --- dlls/rpcrt4/ndr_marshall.c 1 Jun 2005 11:04:03 -0000 1.24 +++ dlls/rpcrt4/ndr_marshall.c 5 Jun 2005 22:03:11 -0000 @@ -575,19 +575,19 @@ void WINAPI PointerMarshall(PMIDL_STUB_M TRACE("(%p,%p,%p,%p)\n", pStubMsg, Buffer, Pointer, pFormat); TRACE("type=0x%x, attr=", type); dump_pointer_attr(attr); pFormat += 2;
if (attr & RPC_FC_P_SIMPLEPOINTER) desc = pFormat; else desc = pFormat + *(const SHORT*)pFormat;
if (attr & RPC_FC_P_DEREF) {
if (!Pointer)
RpcRaiseException(RPC_X_NULL_REF_POINTER);Pointer = *(unsigned char**)Pointer; TRACE("deref => %p\n", Pointer); }
switch (type) {
- case RPC_FC_RP: /* ref pointer (always non-null) */
-#if 0 /* this causes problems for InstallShield so is disabled - we need more tests */
- if (!Pointer)
RpcRaiseException(RPC_X_NULL_REF_POINTER);-#endif
- case RPC_FC_RP: /* ref pointer (always non-null but may point to null) */ break; case RPC_FC_UP: /* unique pointer */ case RPC_FC_OP: /* object pointer - same as unique here */
This looks wrong. A ref pointer shouldn't be treated as a unique pointer in any circumstances AFAIK. I'll add this case to my mini test suite to confirm or deny this hypothesis.
On Mon, 2005-06-06 at 10:26 -0500, Robert Shearman wrote:
switch (type) {
- case RPC_FC_RP: /* ref pointer (always non-null) */
-#if 0 /* this causes problems for InstallShield so is disabled - we
need more tests */
- if (!Pointer)
RpcRaiseException(RPC_X_NULL_REF_POINTER);-#endif
- case RPC_FC_RP: /* ref pointer (always non-null but may point to
null) */
break;case RPC_FC_UP: /* unique pointer */ case RPC_FC_OP: /* object pointer - same as unique here */
This looks wrong. A ref pointer shouldn't be treated as a unique pointer in any circumstances AFAIK. I'll add this case to my mini test suite to confirm or deny this hypothesis.
This is wire-sizing, the full code is:
switch (type) { case RPC_FC_RP: case RPC_FC_OP: case RPC_FC_UP: pStubMsg->BufferLength += 4; /* NULL pointer has no further representation */ if (!Pointer) return; break; case RPC_FC_FP: default: FIXME("unhandled ptr type=%02x\n", type); RpcRaiseException(RPC_X_BAD_STUB_DATA); }
m = NdrBufferSizer[*desc & NDR_TABLE_MASK]; if (m) m(pStubMsg, Pointer, desc); else FIXME("no buffersizer for data type=%02x\n", *desc);
In this case, we need to reserve space for a refptr on the wire to be able to tell the difference between NULL and non-NULL. So it reserves 4 bytes in the buffer.
Mike Hearn wrote:
On Mon, 2005-06-06 at 10:26 -0500, Robert Shearman wrote:
switch (type) {
- case RPC_FC_RP: /* ref pointer (always non-null) */
-#if 0 /* this causes problems for InstallShield so is disabled - we
need more tests */
- if (!Pointer)
RpcRaiseException(RPC_X_NULL_REF_POINTER);-#endif
- case RPC_FC_RP: /* ref pointer (always non-null but may point to
null) */
break; case RPC_FC_UP: /* unique pointer */ case RPC_FC_OP: /* object pointer - same as unique here */
This looks wrong. A ref pointer shouldn't be treated as a unique pointer in any circumstances AFAIK. I'll add this case to my mini test suite to confirm or deny this hypothesis.
This is wire-sizing, the full code is:
switch (type) { case RPC_FC_RP: case RPC_FC_OP: case RPC_FC_UP: pStubMsg->BufferLength += 4; /* NULL pointer has no further representation */ if (!Pointer) return; break; case RPC_FC_FP: default: FIXME("unhandled ptr type=%02x\n", type); RpcRaiseException(RPC_X_BAD_STUB_DATA); }
m = NdrBufferSizer[*desc & NDR_TABLE_MASK]; if (m) m(pStubMsg, Pointer, desc); else FIXME("no buffersizer for data type=%02x\n", *desc);
In this case, we need to reserve space for a refptr on the wire to be able to tell the difference between NULL and non-NULL. So it reserves 4 bytes in the buffer.
Exactly. A refptr shouldn't have those extra 4 bytes because it should never be NULL. I would be very surprised if Microsoft have chosen to be inconsistent here.
On Mon, 2005-06-06 at 11:11 -0500, Robert Shearman wrote:
Exactly. A refptr shouldn't have those extra 4 bytes because it should never be NULL. I would be very surprised if Microsoft have chosen to be inconsistent here.
Refptrs can't be NULL but their contents once de-referenced can be, the problem here is that we have a function like this:
Foo([in] ?? a, [in] ?? b, [out] IBar **bar);
bar is a refptr, it's not NULL. But the contents once dereferenced are (ie, it's returning a null ifptr). We aren't marshalling this correctly at the moment, instead we don't allocate any space in the buffer for the refptr itself and just dereference it. Then when we discover it's NULL, we don't marshal anything (because there's nothing to demarshal) so we lose sync on the receiving end.
thanks -mike
-
Mike Hearn -
Robert Shearman