Add toString to classes extending ByReference

[dbwiddis]

Fixes #1179

[dbwiddis]

I think the test is failing due to a bug in CHARByReference. I'll post on the mailing list.

[dbwiddis]

Hmm, ULONGLONG's SIZE attribute is probably wrong. I'll need help on the correct fix there.

[dbwiddis]

Finally, all the tests pass. Ready for review!

[matthiasblaesing]

Thank you - this looks sane - however I left some comments inline, which might be a good addition. Please have a look.

[matthiasblaesing]

The toString for NativeLong fails on platforms where native long is 32bit. This is the case on windows (see the appveyor build: https://ci.appveyor.com/project/matthiasblaesing/jna/builds/32826833#L940) and is also reproducible on 32bit JDKs on linux. The hex representation seems to be 64 bit wide.

[dbwiddis]

Well, that's a weird one. Originally I had an if-else because I was duplicating the value, and it worked. With only one value I shortened it to the ternary operator. But apparently String.format() decides on the integer type before applying the operator:

        int foo = -42;
        long bar = -42L;
        System.out.format("int: %x%n", foo);
        System.out.format("long: %x%n", bar);
        System.out.format("int?: %x%n", true ? foo : bar);

Output:

int: ffffffd6
long: ffffffffffffffd6
int?: ffffffffffffffd6

So I guess I'll go back to the if-else.

[dbwiddis]

(Also I've now learned about compile-time polymorphism. Yay.)

[matthiasblaesing]

Could you have a look at this: matthiasblaesing@4b24046? This fetches the value via reflection and thus will not need modifications to the implementation. I try to keep changes to the API to a minimum.

[dbwiddis]

Interesting approach! I like it. Three comments:

1. Reflection performance overhead. Probably not important as toString() is unlikely to be used in a performance sensitive way.
2. I'd hope we can more narrowly class the exception. NoSuchMethodException looks like what we're trying to handle with the "contract violated" annotation. But would SecurityException prevent this access if the implementing class's method was private or package private? If we go this route we should more explicitly document user requirements for these methods.
3. This does end up boxing primitive returns. Probably not a big deal, as users who prefer int to Integer in the toString can override it (like we have).

[matthiasblaesing]

Thank you. For the build/unittest failure: Multicatch is not supported on JDK 6 (yes we are that backwards compatible).

For the performance standpoint: toString() was asked to be overriden to make debugging easier. If that becomes a performance bottleneck, we need to reevaluate, but I doubt it.

The broad exception catch was done to make the code more readable (see first paragraph).

The boxing of primitives is also present in the original implementation - the call to String toString(Object o) also goes through boxing. The important core classes are already covered with explicit class names, so I would consider this acceptable.

[matthiasblaesing]

Oh and another unittest failure:

 [junit] Testsuite: com.sun.jna.platform.ByReferencePlatformToStringTest
    [junit] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.265 sec
    [junit] 
    [junit] Testcase: testPlatformToStrings(com.sun.jna.platform.ByReferencePlatformToStringTest):	FAILED
    [junit] Incorrect type prefix expected:<[P]ACL> but was:<[]ACL>
    [junit] junit.framework.AssertionFailedError: Incorrect type prefix expected:<[P]ACL> but was:<[]ACL>
    [junit] 	at com.sun.jna.platform.ByReferencePlatformToStringTest.parseAndTest(ByReferencePlatformToStringTest.java:196)
    [junit] 	at com.sun.jna.platform.ByReferencePlatformToStringTest.testPlatformToStrings(ByReferencePlatformToStringTest.java:126)

PACLByReference should have been ACLByReference. In the headers this is named PACL which is a pointer to ACL

[dbwiddis]

Thanks for the compatibility note. I was confused by the test failures! Will fix.

[dbwiddis]

I'm confused on the PACL failure, as I thought I'd fixed that a while ago. The current code is only supposed to test ACL:
parseAndTest(paclbr.toString(), "ACL", "WinNT$ACL(native");.
And the error message above says expected:<[P]ACL> but I can't see where that expectation comes from.

[dbwiddis]

And I don't think the existing test failures have anything to do with this code. Seeing them on other unrelated PRs too.

[matthiasblaesing]

Rebased and merged - thank you.