Skip to content

Commit

Permalink
SF 3435567
Browse files Browse the repository at this point in the history
 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.
  • Loading branch information
clay_shooter committed Dec 11, 2011
1 parent b3d6de4 commit 745b484
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 5 deletions.
20 changes: 20 additions & 0 deletions jacob/docs/ReleaseNotes.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,26 @@ <h3>Tracked Changes</h3>
<tr>
<td colspan="2"><b>Patches</b></td>
</tr>
<tr>
<td width="13%" valign="top">3435567</td>
<td width="87%" valign="top">(M2)Add HRESULT to error message when unknown COM error raised in Dispatch</td>
</tr>
<tr>
<td width="13%" valign="top">&nbsp;</td>
<td width="87%" valign="top">(M2)Added debug info to advise failure messages.</td>
</tr>
<tr>
<td width="13%" valign="top">&nbsp;</td>
<td width="87%" valign="top">(M2)Added support for null dispatch object in putVariantDispatch.</td>
</tr>
<tr>
<td width="13%" valign="top">&nbsp;</td>
<td width="87%" valign="top">(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</td>
</tr>
<tr>
<td width="13%" valign="top">&nbsp;</td>
<td width="87%" valign="top">(M2)Variant.getString() now returns null for NULL or EMPTY Variants instead of throwing exception.</td>
</tr>
<tr>
<td width="13%" valign="top">3377279</td>
<td width="87%" valign="top">(M1)Fix possible exception. Added initializing Variant used to retrieve
Expand Down
9 changes: 8 additions & 1 deletion jacob/jni/Dispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion jacob/jni/EventProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);
Expand Down
12 changes: 10 additions & 2 deletions jacob/jni/Variant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down
6 changes: 5 additions & 1 deletion jacob/src/com/jacob/com/Variant.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand Down
13 changes: 13 additions & 0 deletions jacob/unittest/com/jacob/com/VariantTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}

0 comments on commit 745b484

Please sign in to comment.