Skip to content

Commit ca585ca

Browse files
twalltwall
committed
Fix issue #475 
1 parent 5d4e193 commit ca585ca

File tree

5 files changed

+51
-84
lines changed

5 files changed

+51
-84
lines changed

‎CHANGES.md

+1
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ Bug Fixes
7373
* [#404](https://github.com/twall/jna/pull/404): Fix `VARIANT` constructors for `int`, `short`, and `long` - [@lwahonen](https://github.com/lwahonen).
7474
* [#420](https://github.com/twall/jna/pull/420): Fix structure leaving always one element in ThreadLocal set - [@sjappig](https://github.com/sjappig).
7575
* [#467](https://github.com/twall/jna/issues/467): Fix TypeMapper usage with direct-mapped libraries converting primitives to Java objects (specifically enums) - [@twall](https://github.com/twall).
76+
* [#475](https://github.com/twall/jna/issues/475): Avoid modifying native memory in `Structure.equals()/hashCode()`- [@twall](https://github.com/twall).
7677

7778
Release 4.1
7879
===========

‎src/com/sun/jna/Structure.java

+25-28
Original file line numberDiff line numberDiff line change
@@ -1462,42 +1462,39 @@ private Class baseClass() {
14621462
return getClass();
14631463
}
14641464

1465-
/** This structure is equal to another based on the same data type
1466-
* and memory contents.
1465+
/** Return whether the given Structure's backing data is identical to
1466+
* this one.
14671467
*/
1468-
public boolean equals(Object o) {
1469-
if (o == this) {
1470-
return true;
1471-
}
1472-
if (!(o instanceof Structure)) {
1473-
return false;
1474-
}
1475-
if (o.getClass() != getClass()
1476-
&& ((Structure)o).baseClass() != baseClass()) {
1477-
return false;
1478-
}
1479-
Structure s = (Structure)o;
1480-
if (s.getPointer().equals(getPointer())) {
1468+
public boolean dataEquals(Structure s) {
1469+
byte[] data = s.getPointer().getByteArray(0, s.size());
1470+
byte[] ref = getPointer().getByteArray(0, size());
1471+
if (data.length == ref.length) {
1472+
for (int i=0;i < data.length;i++) {
1473+
if (data[i] != ref[i]) {
1474+
System.out.println("byte mismatch at offset " + i);
1475+
return false;
1476+
}
1477+
}
14811478
return true;
14821479
}
1483-
if (s.size() == size()) {
1484-
clear(); write();
1485-
byte[] buf = getPointer().getByteArray(0, size());
1486-
s.clear(); s.write();
1487-
byte[] sbuf = s.getPointer().getByteArray(0, s.size());
1488-
return Arrays.equals(buf, sbuf);
1489-
}
14901480
return false;
14911481
}
14921482

1493-
/** Since {@link #equals} depends on the contents of memory, use that
1494-
* as the basis for the hash code.
1483+
/** Structures are equal if they share the same pointer. */
1484+
public boolean equals(Object o) {
1485+
return o instanceof Structure
1486+
&& o.getClass() == getClass()
1487+
&& ((Structure)o).getPointer().equals(getPointer());
1488+
}
1489+
1490+
/** Use the underlying memory pointer's hash code.
14951491
*/
14961492
public int hashCode() {
1497-
clear(); write();
1498-
Adler32 code = new Adler32();
1499-
code.update(getPointer().getByteArray(0, size()));
1500-
return (int)code.getValue();
1493+
Pointer p = getPointer();
1494+
if (p != null) {
1495+
return getPointer().hashCode();
1496+
}
1497+
return getClass().hashCode();
15011498
}
15021499

15031500
/** Cache native type information for use in native code. */

‎test/com/sun/jna/ArgumentsMarshalTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -565,9 +565,9 @@ public void testStructureByReferenceArrayArgument() {
565565
null,
566566
new CheckFieldAlignment.ByReference(),
567567
};
568-
assertEquals("Wrong value returned (0)", args[0], lib.returnPointerArrayElement(args, 0));
568+
assertTrue("Wrong value returned (0)", args[0].dataEquals(lib.returnPointerArrayElement(args, 0)));
569569
assertNull("Wrong value returned (1)", lib.returnPointerArrayElement(args, 1));
570-
assertEquals("Wrong value returned (2)", args[2], lib.returnPointerArrayElement(args, 2));
570+
assertTrue("Wrong value returned (2)", args[2].dataEquals(lib.returnPointerArrayElement(args, 2)));
571571
assertNull("Native array should be null terminated", lib.returnPointerArrayElement(args, 3));
572572
}
573573

‎test/com/sun/jna/CallbacksTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -741,8 +741,8 @@ public TestStructure.ByValue callback(TestStructure.ByValue s) {
741741
+ arg[0].getPointer(), arg[0].getPointer() instanceof Memory);
742742
assertTrue("ByValue result should own its own memory, instead was "
743743
+ result.getPointer(), result.getPointer() instanceof Memory);
744-
assertEquals("Wrong value for callback argument", s, arg[0]);
745-
assertEquals("Wrong value for callback result", s, result);
744+
assertTrue("Wrong value for callback argument", s.dataEquals(arg[0]));
745+
assertTrue("Wrong value for callback result", s.dataEquals(result));
746746
}
747747

748748
public void testUnionByValueCallbackArgument() throws Exception{

‎test/com/sun/jna/StructureTest.java

+21-52
Original file line numberDiff line numberDiff line change
@@ -895,6 +895,12 @@ protected List getFieldOrder() {
895895
assertEquals("Non-volatile field should be written", 1, s.getPointer().getInt(4));
896896
s.writeField("counter");
897897
assertEquals("Explicit volatile field write failed", 1, s.getPointer().getInt(0));
898+
899+
s.equals(s);
900+
assertEquals("Structure equals should leave volatile field unchanged", 1, s.getPointer().getInt(0));
901+
902+
s.hashCode();
903+
assertEquals("Structure equals should leave volatile field unchanged", 1, s.getPointer().getInt(0));
898904
}
899905

900906
public static class StructureWithPointers extends Structure {
@@ -1542,53 +1548,27 @@ class TestStructure extends Structure {
15421548
protected List getFieldOrder() {
15431549
return Arrays.asList(new String[] { "first", "second", "third" });
15441550
}
1551+
public TestStructure() { }
1552+
public TestStructure(Pointer p) { super(p); }
15451553
}
15461554
OtherStructure s0 = new OtherStructure();
15471555
TestStructure s1 = new TestStructure();
1548-
TestStructure s2 = new TestStructure();
1549-
TestStructure s3 = new TestStructure();
1550-
int VALUE = 99;
1551-
s1.first = s2.first = s3.first = VALUE;
1556+
TestStructure s2 = new TestStructure(s1.getPointer());
1557+
TestStructure s3 = new TestStructure(s2.getPointer());
1558+
TestStructure s4 = new TestStructure();
15521559

15531560
assertFalse("Structures of different classes with same fields are not equal", s1.equals(s0));
15541561
assertFalse("Structures of different classes with same fields are not equal (reflexive)", s0.equals(s1));
15551562

15561563
assertFalse("Compare to null failed", s1.equals(null));
15571564
assertTrue("Equals is not reflexive", s1.equals(s1));
1558-
assertTrue("Equals failed on identical structures", s1.equals(s2));
1565+
assertTrue("Equals failed on structures with same pointer", s1.equals(s2));
15591566
assertTrue("Equals is not symmetric", s2.equals(s1));
15601567
assertTrue("Equals is not transitive", s1.equals(s2) && s2.equals(s3) && s1.equals(s3));
1561-
1562-
1568+
assertFalse("Compare to different structure failed", s1.equals(s4));
15631569
}
15641570

1565-
public void testStructureEqualsByValueByReference() {
1566-
class TestStructure extends Structure {
1567-
public int first;
1568-
public int[] second = new int[4];
1569-
public Pointer[] third = new Pointer[4];
1570-
protected List getFieldOrder() {
1571-
return Arrays.asList(new String[] { "first", "second", "third" });
1572-
}
1573-
}
1574-
class ByReference extends TestStructure implements Structure.ByReference { }
1575-
class ByValue extends TestStructure implements Structure.ByValue { }
1576-
TestStructure s1 = new TestStructure();
1577-
TestStructure s2 = new ByReference();
1578-
TestStructure s3 = new ByValue();
1579-
int VALUE = 99;
1580-
s1.first = s2.first = s3.first = VALUE;
1581-
1582-
assertTrue("Equals failed on identical ByReference", s1.equals(s2));
1583-
assertTrue("Equals is not symmetric (ByReference)", s2.equals(s1));
1584-
assertTrue("Equals failed on identical ByValue", s1.equals(s3));
1585-
assertTrue("Equals is not symmetric (ByValue)", s3.equals(s1));
1586-
assertTrue("Equals is not transitive (ByReference/ByValue)", s1.equals(s2) && s2.equals(s3) && s1.equals(s3));
1587-
1588-
1589-
}
1590-
1591-
public void testStructureHashCodeMatchesEqualsTrue() {
1571+
public void testStructureHashCodeMatchesWhenEqual() {
15921572
class TestStructure extends Structure {
15931573
public int first;
15941574
protected List getFieldOrder() {
@@ -1597,26 +1577,15 @@ protected List getFieldOrder() {
15971577
}
15981578
TestStructure s1 = new TestStructure();
15991579
TestStructure s2 = new TestStructure();
1600-
s1.first = s2.first = 0x12345678;
1601-
assertEquals("hashCode should match when structures equal",
1602-
s1.hashCode(), s2.hashCode());
1603-
}
1580+
assertFalse("hashCode should be different for two different structures", s1.hashCode() == s2.hashCode());
16041581

1605-
public void testStructureEqualsIgnoresPadding() {
1606-
class TestStructure extends Structure {
1607-
public byte first;
1608-
public int second;
1609-
protected List getFieldOrder() {
1610-
return Arrays.asList(new String[] { "first", "second" });
1611-
}
1612-
}
1613-
TestStructure s1 = new TestStructure();
1614-
TestStructure s2 = new TestStructure();
1582+
s2.useMemory(s1.getPointer());
1583+
assertEquals("hashCode should match when structures have same pointer",
1584+
s1.hashCode(), s2.hashCode());
16151585

1616-
// Make padding bits non-zero
1617-
s2.getPointer().setInt(0, -1);
1618-
s2.write();
1619-
assertTrue("Structure equals should ignore padding", s1.equals(s2));
1586+
s2.useMemory(s1.getPointer());
1587+
assertEquals("hashCode should match when structures have same pointer",
1588+
s1.hashCode(), s2.hashCode());
16201589
}
16211590

16221591
public void testRecursiveWrite() {

0 commit comments

Comments
 (0)