This is an automated email from the ASF dual-hosted git repository. leginee pushed a commit to branch win10-msvc-trunk in repository https://gitbox.apache.org/repos/asf/openoffice.git
commit f1b4f0d60bff2fd6d79ab0a44201ec7312974be5 Author: Peter Kovacs <[email protected]> AuthorDate: Fri Aug 21 22:24:54 2026 +0200 cli_ure: the CLI bridgetest passes -- three more defects, all one mistake [cli bridgetest] 1. C# client calls C# object ok [cli bridgetest] 2. C++ client (native) calls C# object ok [cli bridgetest] 3. C# client calls C++ object (native) ok [cli bridgetest] 4. Visual Basic client calls C++ object ok [cli bridgetest] 6. CLI C++ client calls C++ object ok ====> all tests ok. THE SAME MISTAKE THREE TIMES. I fixed "handle != 0" as a COMPARISON in 10b60aa28b and stopped there. It is the same trap in an ASSIGNMENT, and in a property access, and neither is caught by sweeping bare identifiers: *cli_data = NULL; // map_to_cli, interface case xLBT->Interface = 0; // the test's own null-reference check bRet = (aRet->Interface == 0); // ...and its verification Assigning 0 to a System::Object^ BOXES it into an Int32. It does not store null. In the bridge that meant a null UNO interface arrived on the CLI side as a boxed integer; mapping it back, "nullptr == cli_data" was false, so the bridge built a proxy FOR THAT INTEGER and a null interface came back non-null -- a different address every run. Finding it needed the data, not the code. A normalized diff of cli_data.cxx against the MC++ original (strip ^, gcnew, ::typeid, pin_ptr, and diff what is left) accounted for every remaining line and turned up nothing, which was the useful result: it ruled the file out and sent me to the values instead. A probe in the native test's equals() showed the sequence lengths matched but the contents did not; per-element, element 1 -- the deliberately EMPTY one -- failed on "### interface does not match!"; printing the pointers gave iface1=00000000 iface2=00BF9714, and the shape of the bug was obvious. Note how well this bug hid. equals(TestData) short-circuits on the sequence comparison BEFORE the per-field checks, so nothing said "interface"; and the test's own null check is one of the only two assertions in performTest with no check() message, so stage 6 failed as a bare "standard test failed". ALSO, and separately: a polymorphic struct's type parameter came out as the text "True". looked up UNO name: []test.testtools.bridgetest.TestPolyStruct<True> StringBuilder::Append was being handed OUString::getStr(), a sal_Unicode const*. There is no Append(sal_Unicode const*); the only overload an "unsigned short const*" converts to is Append(bool), by the ordinary pointer-to-bool conversion. MC++ resolved it differently. Now it maps the OUString to a System::String^ first. That one was diagnosed by making the error message say which UNO name it had looked up -- "could not map type: X" never said, and for a polymorphic struct that name IS the question. The message keeps that now. AND: sequences of unsigned integers came back signed. Marshal::Copy has no unsigned overloads, so the original allocated a UInt16[] and cast it to Int16[] FOR THE COPY CALL ONLY, handing back the unsigned array. The port changed the ALLOCATION type instead -- the same shape as the Boolean[] bug in 10b60aa28b -- so callers got short[] where the signature says ushort[]. Note safe_cast rejects the array-to-array form at compile time even though the CLR permits it at runtime; go through Object^. What I nearly broke: XBridgeTest has "[attribute] long RuntimeException", and "xLBT->RuntimeException = 0" is an ordinary integer assignment that must stay. Check the IDL before treating "= 0" as a null. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01W7pjcp2sXU1HaUwXT7kc29 --- main/cli_ure/source/uno_bridge/cli_data.cxx | 61 ++++++++++++++++++---- main/cli_ure/source/uno_bridge/cli_environment.cxx | 2 +- main/cli_ure/source/uno_bridge/cli_proxy.cxx | 2 +- .../source/bridgetest/cli/cli_cpp_bridgetest.cxx | 10 ++-- 4 files changed, 61 insertions(+), 14 deletions(-) diff --git a/main/cli_ure/source/uno_bridge/cli_data.cxx b/main/cli_ure/source/uno_bridge/cli_data.cxx index 4e3a1a3a89..018cedab06 100644 --- a/main/cli_ure/source/uno_bridge/cli_data.cxx +++ b/main/cli_ure/source/uno_bridge/cli_data.cxx @@ -326,6 +326,10 @@ System::Type ^ mapUnoType(typelib_TypeDescriptionReference const * pTD) typelib_TypeDescriptionReference* mapCliType(System::Type ^ cliType) { typelib_TypeDescriptionReference* retVal= NULL; + // Kept for the error message: "could not map type: X" alone does not say + // which UNO name was looked up, and for a polymorphic struct that name is + // the whole question. + OUString usTypeName; if (cliType == nullptr) { retVal = * typelib_static_type_getByTypeClass( @@ -440,7 +444,6 @@ typelib_TypeDescriptionReference* mapCliType(System::Type ^ cliType) //struct, interfaces, sequences else { - OUString usTypeName; uno::PolymorphicType ^ poly = dynamic_cast<uno::PolymorphicType ^>(cliType); if (poly != nullptr) usTypeName = mapCliTypeName( poly->PolymorphicName); @@ -463,6 +466,12 @@ typelib_TypeDescriptionReference* mapCliType(System::Type ^ cliType) RTL_CONSTASCII_STRINGPARAM("[cli_uno bridge] mapCliType():" "could not map type: ") ); buf.append(mapCliString(cliType->FullName)); + if (usTypeName.getLength()) + { + buf.appendAscii( RTL_CONSTASCII_STRINGPARAM(" (looked up UNO name: ") ); + buf.append(usTypeName); + buf.appendAscii( RTL_CONSTASCII_STRINGPARAM(")") ); + } throw BridgeRuntimeError( buf.makeStringAndClear() ); } return retVal; @@ -598,7 +607,14 @@ System::String ^ mapPolymorphicName(System::String ^ unoName, bool bCliToUno) index = cur; if (bCliToUno) { - builder->Append( mapCliTypeName(sParam).getStr()); + // NOT Append(OUString::getStr()). getStr() hands back a + // sal_Unicode const*, and the only Append overload a + // "unsigned short const*" converts to is Append(bool) -- via + // the ordinary pointer-to-bool conversion. The type + // parameter then comes out as the text "True" and the + // resulting UNO name matches nothing: + // []test.testtools.bridgetest.TestPolyStruct<True> + builder->Append( mapUnoString( mapCliTypeName(sParam).pData ) ); } else { @@ -1813,8 +1829,15 @@ void Bridge::map_to_cli( } case typelib_TypeClass_UNSIGNED_SHORT: { - cli::array< System::Int16 > ^ arUInt16 = gcnew cli::array< System::Int16 >( nElements ); - sri::Marshal::Copy( System::IntPtr( (void*) &seq->elements ), arUInt16, + // The caller gets an unsigned array -- allocate THAT, and cast + // only for the Copy call, which has no unsigned overload. An + // array of unsigned and one of signed of the same size are + // castclass compatible, so the cast is a no-op at runtime. + cli::array< System::UInt16 > ^ arUInt16 = + gcnew cli::array< System::UInt16 >( nElements ); + sri::Marshal::Copy( System::IntPtr( (void*) &seq->elements ), + safe_cast< cli::array< System::Int16 > ^ >( + (System::Object ^) arUInt16 ), 0, nElements); *cli_data= arUInt16; break; @@ -1828,8 +1851,15 @@ void Bridge::map_to_cli( } case typelib_TypeClass_UNSIGNED_LONG: { - cli::array< System::Int32 > ^ arUInt32 = gcnew cli::array< System::Int32 >( nElements ); - sri::Marshal::Copy( System::IntPtr( (void*) &seq->elements ), arUInt32, + // The caller gets an unsigned array -- allocate THAT, and cast + // only for the Copy call, which has no unsigned overload. An + // array of unsigned and one of signed of the same size are + // castclass compatible, so the cast is a no-op at runtime. + cli::array< System::UInt32 > ^ arUInt32 = + gcnew cli::array< System::UInt32 >( nElements ); + sri::Marshal::Copy( System::IntPtr( (void*) &seq->elements ), + safe_cast< cli::array< System::Int32 > ^ >( + (System::Object ^) arUInt32 ), 0, nElements); *cli_data= arUInt32; break; @@ -1843,8 +1873,16 @@ void Bridge::map_to_cli( } case typelib_TypeClass_UNSIGNED_HYPER: { - cli::array< System::Int64 > ^ arUInt64 = gcnew cli::array< System::Int64 >( nElements ); - sri::Marshal::Copy( System::IntPtr( (void*) &seq->elements ), arUInt64, 0, nElements); + // The caller gets an unsigned array -- allocate THAT, and cast + // only for the Copy call, which has no unsigned overload. An + // array of unsigned and one of signed of the same size are + // castclass compatible, so the cast is a no-op at runtime. + cli::array< System::UInt64 > ^ arUInt64 = + gcnew cli::array< System::UInt64 >( nElements ); + sri::Marshal::Copy( System::IntPtr( (void*) &seq->elements ), + safe_cast< cli::array< System::Int64 > ^ >( + (System::Object ^) arUInt64 ), + 0, nElements); *cli_data= arUInt64; break; } @@ -2006,7 +2044,12 @@ void Bridge::map_to_cli( typelib_InterfaceTypeDescription*>(td.get())) ; } else - *cli_data= NULL; + // NOT NULL. *cli_data is a System::Object^, and assigning the + // literal 0 to one BOXES it into an Int32 rather than storing a + // null reference. The caller then holds a boxed integer where the + // signature says an interface; mapping it back builds a proxy FOR + // THAT INTEGER, so a null interface returns non-null. + *cli_data= nullptr; break; } default: diff --git a/main/cli_ure/source/uno_bridge/cli_environment.cxx b/main/cli_ure/source/uno_bridge/cli_environment.cxx index 8d6c513dca..011a2d70a6 100644 --- a/main/cli_ure/source/uno_bridge/cli_environment.cxx +++ b/main/cli_ure/source/uno_bridge/cli_environment.cxx @@ -119,7 +119,7 @@ System::Object ^ Cli_environment::getRegisteredInterface(System::String ^ oid, System::Type ^ type) { //try if it is a UNO interface - System::Object ^ ret = NULL; + System::Object ^ ret = nullptr; ret = m_objects[ oid ]; if (! ret) { diff --git a/main/cli_ure/source/uno_bridge/cli_proxy.cxx b/main/cli_ure/source/uno_bridge/cli_proxy.cxx index a4746b98b4..d622111af6 100644 --- a/main/cli_ure/source/uno_bridge/cli_proxy.cxx +++ b/main/cli_ure/source/uno_bridge/cli_proxy.cxx @@ -334,7 +334,7 @@ srrm::IMessage ^ UnoInterfaceProxy::invokeObject( srrm::LogicalCallContext ^ context, srrm::IMethodCallMessage ^ mcm) { - System::Object ^ retMethod = 0; + System::Object ^ retMethod = nullptr; System::String ^ sMethod = static_cast<System::String ^> (props[ m_methodNameString ]); cli::array< System::Object ^ > ^ args = safe_cast< cli::array< System::Object ^ > ^ >( diff --git a/main/testtools/source/bridgetest/cli/cli_cpp_bridgetest.cxx b/main/testtools/source/bridgetest/cli/cli_cpp_bridgetest.cxx index cbc656e527..ab65680c5c 100644 --- a/main/testtools/source/bridgetest/cli/cli_cpp_bridgetest.cxx +++ b/main/testtools/source/bridgetest/cli/cli_cpp_bridgetest.cxx @@ -523,10 +523,14 @@ static bool performTest(XBridgeTest ^ xLBT) bRet = (compareData( aData, aRet ) && compareData( aData, aRet2 )) && bRet ; - // check setting of null reference - xLBT->Interface = 0; + // check setting of null reference. XInterface maps to Object^, and + // "= 0" on one BOXES the literal into an Int32 instead of storing + // null -- so the property ends up non-null and the comparison below + // is false whatever happens. Neither has a check() message, which is + // why this failed as a bare "standard test failed". + xLBT->Interface = nullptr; aRet->Interface = xLBT->Interface; - bRet = (aRet->Interface == 0) && bRet; + bRet = (aRet->Interface == nullptr) && bRet; }
