From 745b48484fb176e0adc3b61639e3185766bb3cab Mon Sep 17 00:00:00 2001 From: clay_shooter <> Date: Sun, 11 Dec 2011 16:57:29 +0000 Subject: [PATCH] SF 3435567 Possibility to pass a NULL Dispatch pointer Fix a memory leak (detected with Glowcode) in Variant.cpp/zeroVariant function Variant.toString improvement to handle NULL cases Adds the error code to the message when "an unknown COM error has occurred" Added debug info to EventProxy advise registration failure message. --- jacob/docs/ReleaseNotes.html | 20 +++++++++++++++++++ jacob/jni/Dispatch.cpp | 9 ++++++++- jacob/jni/EventProxy.cpp | 6 +++++- jacob/jni/Variant.cpp | 12 +++++++++-- jacob/src/com/jacob/com/Variant.java | 6 +++++- jacob/unittest/com/jacob/com/VariantTest.java | 13 ++++++++++++ 6 files changed, 61 insertions(+), 5 deletions(-) diff --git a/jacob/docs/ReleaseNotes.html b/jacob/docs/ReleaseNotes.html index 054b85b..236b4c4 100644 --- a/jacob/docs/ReleaseNotes.html +++ b/jacob/docs/ReleaseNotes.html @@ -26,6 +26,26 @@

Tracked Changes

Patches + + 3435567 + (M2)Add HRESULT to error message when unknown COM error raised in Dispatch + + +   + (M2)Added debug info to advise failure messages. + + +   + (M2)Added support for null dispatch object in putVariantDispatch. + + +   + (M2)Fixed memory leak in Variant.cpp zeroVariant method possibly related to previous fix proposed in SF 1689061 but never implemented. I guess we should fix it since people keep pointing it out + + +   + (M2)Variant.getString() now returns null for NULL or EMPTY Variants instead of throwing exception. + 3377279 (M1)Fix possible exception. Added initializing Variant used to retrieve diff --git a/jacob/jni/Dispatch.cpp b/jacob/jni/Dispatch.cpp index a08153b..2a222d8 100644 --- a/jacob/jni/Dispatch.cpp +++ b/jacob/jni/Dispatch.cpp @@ -342,8 +342,15 @@ static wchar_t* CreateErrorMsgFromResult(HRESULT inResult) msg = (wchar_t*) ::LocalAlloc(LPTR, bufferLength); wcscpy_s(msg, bufferLength, message_text); } + // SF 3435567 add HRESULT to error message + size_t bufferLength = (wcslen(msg) + 100) * sizeof(wchar_t); + wchar_t* plus = (wchar_t*) ::LocalAlloc(LPTR, bufferLength); + // Had to force this to wide/unicode. We must be missing a macro or setting somewhere + wsprintfW(plus, L"%x / %s", inResult, msg); - return msg; + ::LocalFree(msg); + + return plus; } static wchar_t* CreateErrorMsgFromInfo(HRESULT inResult, EXCEPINFO* ioInfo, diff --git a/jacob/jni/EventProxy.cpp b/jacob/jni/EventProxy.cpp index 7c035b6..023b316 100644 --- a/jacob/jni/EventProxy.cpp +++ b/jacob/jni/EventProxy.cpp @@ -50,7 +50,10 @@ void EventProxy::Connect(JNIEnv *env) { connected = 1; } else { connected = 0; - ThrowComFail(env, "Advise failed", hr); + // SF 3435567 added debug info to advise failed message + char tmp[256]; + sprintf_s( tmp, 256, "Advise failed with %x (CONNECT_E_ADVISELIMIT is %x)", (int)hr, (int)(CONNECT_E_ADVISELIMIT) ); + ThrowComFail(env, tmp, hr); } } @@ -240,6 +243,7 @@ STDMETHODIMP EventProxy::Invoke(DISPID dispID, REFIID riid, VARIANT *com = &pDispParams->rgvarg[i]; convertJavaVariant(java, com); // SF 1689061 change not accepted but put in as comment for later reminder + // note that a related fix has been submitted in SF 3435567 to do this in zeroVariant() method //Java_com_jacob_com_Variant_release(env, arg); zeroVariant(env, arg); env->DeleteLocalRef(arg); diff --git a/jacob/jni/Variant.cpp b/jacob/jni/Variant.cpp index 88cce9c..c9af0cb 100644 --- a/jacob/jni/Variant.cpp +++ b/jacob/jni/Variant.cpp @@ -95,6 +95,12 @@ JNIEXPORT void JNICALL Java_com_jacob_com_Variant_init */ void zeroVariant(JNIEnv *env, jobject _this) { + // sf 3435567 Fix a memory leak (detected with Glowcode) in Variant.cpp/zeroVariant function + // does this code conflict with the function/method documentation now? + // note that a related fix was proposed in SF 1689061 in EventProxy.cpp but never accepted + VARIANT *v = extractVariant(env, _this); + delete v; + jclass clazz = env->GetObjectClass(_this); jfieldID jf = env->GetFieldID(clazz, VARIANT_FLD, "I"); env->SetIntField(_this, jf, (unsigned int)0); @@ -632,12 +638,14 @@ JNIEXPORT void JNICALL Java_com_jacob_com_Variant_putVariantDispatch { VARIANT *v = extractVariant(env, _this); IDispatch *disp = extractDispatch(env, _that); - if (disp && v) { + if (v) { VariantClear(v); // whatever was there before V_VT(v) = VT_DISPATCH; V_DISPATCH(v) = disp; // I am handing the pointer to COM - disp->AddRef(); + // SF 3435567 support null dispatch pointer + if( disp ) + disp->AddRef(); } else ThrowComFail(env, "putObject failed", -1); } diff --git a/jacob/src/com/jacob/com/Variant.java b/jacob/src/com/jacob/com/Variant.java index 91ae210..0253ce5 100644 --- a/jacob/src/com/jacob/com/Variant.java +++ b/jacob/src/com/jacob/com/Variant.java @@ -795,13 +795,17 @@ public short getShortRef() { /** * - * @return string contents of the variant. + * @return string contents of the variant, null if is of type null or empty * @throws IllegalStateException * if this variant is not of type String */ public String getString() { if (getvt() == Variant.VariantString) { return getVariantString(); + } else if (getvt() == Variant.VariantEmpty) { + return null; + } else if (getvt() == Variant.VariantNull) { + return null; } else { throw new IllegalStateException( "getString() only legal on Variants of type VariantString, not " diff --git a/jacob/unittest/com/jacob/com/VariantTest.java b/jacob/unittest/com/jacob/com/VariantTest.java index 2f4c0ba..4012c1a 100644 --- a/jacob/unittest/com/jacob/com/VariantTest.java +++ b/jacob/unittest/com/jacob/com/VariantTest.java @@ -701,4 +701,17 @@ public void testGetError() { // yeah! can't get dispatch from boolean } } + + /** + * Verify SF 3435567 null and empty behavior change + */ + public void testGetNullString() { + Variant testVariant = new Variant(); + testVariant.putNull(); + assertNull(testVariant.getString()); + testVariant.putEmpty(); + assertNull(testVariant.getString()); + testVariant.putString("dog"); + assertEquals("dog", testVariant.getString()); + } }