Skip to content

Commit

Permalink
java-native-access#630: use of ellipsis in COM methods
Browse files Browse the repository at this point in the history
  • Loading branch information
SevenOf9Sleeper committed Apr 5, 2016
1 parent c83f4b0 commit ea1613d
Showing 1 changed file with 55 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
package com.sun.jna.platform.win32.COM.util;

import java.lang.reflect.InvocationHandler;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Proxy;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeoutException;
Expand Down Expand Up @@ -51,7 +52,6 @@
import com.sun.jna.platform.win32.COM.util.annotation.ComProperty;
import com.sun.jna.ptr.IntByReference;
import com.sun.jna.ptr.PointerByReference;
import java.lang.reflect.InvocationTargetException;

/**
* This object acts as the invocation handler for interfaces annotated with
Expand Down Expand Up @@ -262,7 +262,8 @@ private ConnectionPoint fetchRawConnectionPoint(IID iid) throws InterruptedExcep
return rawCp;
}

public IComEventCallbackCookie advise(Class<?> comEventCallbackInterface,
@Override
public IComEventCallbackCookie advise(Class<?> comEventCallbackInterface,
final IComEventCallbackListener comEventCallbackListener) {
assert COMUtils.comIsInitialized() : "COM not initialized";

Expand Down Expand Up @@ -299,7 +300,8 @@ public IComEventCallbackCookie advise(Class<?> comEventCallbackInterface,
}
}

public void unadvise(Class<?> comEventCallbackInterface, final IComEventCallbackCookie cookie) {
@Override
public void unadvise(Class<?> comEventCallbackInterface, final IComEventCallbackCookie cookie) {
assert COMUtils.comIsInitialized() : "COM not initialized";

try {
Expand Down Expand Up @@ -369,43 +371,60 @@ public <T> T getProperty(Class<T> returnType, DISPID dispID, Object... args) {
return convertAndFreeReturn(result, returnType);
}

@Override
public <T> T invokeMethod(Class<T> returnType, String name, Object... args) {
DISPID dispID = resolveDispId(this.getRawDispatch(), name);
return invokeMethod(returnType, dispID, args);
@Override
public <T> T invokeMethod(Class<T> returnType, String name, Object... args) {
DISPID dispID = resolveDispId(this.getRawDispatch(), name);
return invokeMethod(returnType, dispID, args);
}

@Override
public <T> T invokeMethod(Class<T> returnType, DISPID dispID, Object... argParams) {
assert COMUtils.comIsInitialized() : "COM not initialized";

VARIANT[] vargs;
Object[] args = unfoldWhenEllipsis(argParams);
if (null == args) {
vargs = new VARIANT[0];
}

@Override
public <T> T invokeMethod(Class<T> returnType, DISPID dispID, Object... args) {
assert COMUtils.comIsInitialized() : "COM not initialized";

VARIANT[] vargs;
if (null == args) {
vargs = new VARIANT[0];
} else {
vargs = new VARIANT[args.length];
}
for (int i = 0; i < vargs.length; ++i) {
vargs[i] = Convert.toVariant(args[i]);
}
Variant.VARIANT.ByReference result = new Variant.VARIANT.ByReference();
WinNT.HRESULT hr = this.oleMethod(OleAuto.DISPATCH_METHOD, result, this.getRawDispatch(), dispID, vargs);

for (int i = 0; i < vargs.length; i++) {
// Free value allocated by Convert#toVariant
Convert.free(vargs[i], args[i]);
}

COMUtils.checkRC(hr);
else {
vargs = new VARIANT[args.length];
}
for (int i = 0; i < vargs.length; ++i) {
vargs[i] = Convert.toVariant(args[i]);
}
Variant.VARIANT.ByReference result = new Variant.VARIANT.ByReference();
WinNT.HRESULT hr = this.oleMethod(OleAuto.DISPATCH_METHOD, result, this.getRawDispatch(), dispID, vargs);

return convertAndFreeReturn(result, returnType);
}
for (int i = 0; i < vargs.length; i++) {
// Free value allocated by Convert#toVariant
Convert.free(vargs[i], args[i]);
}

COMUtils.checkRC(hr);

private <T> T convertAndFreeReturn(VARIANT.ByReference result, Class<T> returnType) {
Object jobj = Convert.toJavaObject(result, returnType, factory, false);
Convert.free(result, returnType);
return returnType.cast(jobj);
return convertAndFreeReturn(result, returnType);
}

private Object[] unfoldWhenEllipsis(Object[] argParams) {
if (null == argParams) {
return null;
}
if (argParams.length == 0 || !(argParams[argParams.length - 1] instanceof Object[])) {
return argParams;
}
// when last parameter is Object[] -> unfold the ellipsis:
Object[] ellipsis = (Object[]) argParams[argParams.length - 1];
Object[] args = new Object[argParams.length - 1 + ellipsis.length];
System.arraycopy(argParams, 0, args, 0, argParams.length - 1);
System.arraycopy(ellipsis, 0, args, argParams.length - 1, ellipsis.length);
return args;
}

private <T> T convertAndFreeReturn(VARIANT.ByReference result, Class<T> returnType) {
Object jobj = Convert.toJavaObject(result, returnType, factory, false);
Convert.free(result, returnType);
return returnType.cast(jobj);
}

@Override
public <T> T queryInterface(Class<T> comInterface) throws COMException {
Expand Down

3 comments on commit ea1613d

@matthiasblaesing
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether it is a good idea to introduce another destabilizing factor into the calling logic, but I had a look at this:

  • java reflection can give you info whether the method is a varargs call (Method#isVarArgs), this should eliminate the danger to collide with array arguments
  • please add tests for the changes, exercising the functionality
  • please keep indention in line with the rest of the file or at least don't change lines, that don't need a change, as this adds noise

@matthiasblaesing
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah - and sorry - I thought this would have been a PR already - I did not realize this was WIP....

@SevenOf9Sleeper
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:-)
Want to wait until you have merged your Callback fix ;-)

Sorry for the indentation mix. I will fix it.

Please sign in to comment.