diff --git a/CHANGES.md b/CHANGES.md index 65be473d70..635f1a33a1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -39,6 +39,7 @@ Features * [#595](https://github.com/java-native-access/jna/pull/595): Allow calling COM methods/getters requiring hybrid calling (METHOD+PROPERTYGET) - [@matthiasblaesing](https://github.com/matthiasblaesing). * [#582](https://github.com/java-native-access/jna/pull/582): Mavenize the build process - Phase 1: building the native code via Maven [@lgoldstein](https://github.com/lgoldstein). * [#606](https://github.com/java-native-access/jna/pull/606): Added Kerne32Util method to facilitate checking that calls to LocalFree/GlobalFree are successful [@lgoldstein](https://github.com/lgoldstein). +* [#612](https://github.com/java-native-access/jna/pull/612): Kernel32Util#freeLocal/GlobalMemory always throws Win32Exception if failed [@lgoldstein](https://github.com/lgoldstein). Bug Fixes --------- diff --git a/contrib/platform/src/com/sun/jna/platform/win32/Advapi32Util.java b/contrib/platform/src/com/sun/jna/platform/win32/Advapi32Util.java index 2ab17dfb17..3ed91c6233 100755 --- a/contrib/platform/src/com/sun/jna/platform/win32/Advapi32Util.java +++ b/contrib/platform/src/com/sun/jna/platform/win32/Advapi32Util.java @@ -301,7 +301,7 @@ public static String convertSidToStringSid(PSID sid) { try { return ptr.getWideString(0); } finally { - Kernel32Util.validateFreeLocalMemory(ptr); + Kernel32Util.freeLocalMemory(ptr); } } @@ -323,7 +323,7 @@ public static byte[] convertStringSidToSid(String sidString) { try { return value.getBytes(); } finally { - Kernel32Util.validateFreeLocalMemory(value.getPointer()); + Kernel32Util.freeLocalMemory(value.getPointer()); } } @@ -347,7 +347,7 @@ public static boolean isWellKnownSid(String sidString, int wellKnownSidType) { try { return Advapi32.INSTANCE.IsWellKnownSid(value, wellKnownSidType); } finally { - Kernel32Util.validateFreeLocalMemory(value.getPointer()); + Kernel32Util.freeLocalMemory(value.getPointer()); } } @@ -2214,7 +2214,7 @@ public static Memory getSecurityDescriptorForObject(final String absoluteObjectP memory.write(0, data, 0, nLength); return memory; } finally { - Kernel32Util.validateFreeLocalMemory(secValue); + Kernel32Util.freeLocalMemory(secValue); } } diff --git a/contrib/platform/src/com/sun/jna/platform/win32/Crypt32Util.java b/contrib/platform/src/com/sun/jna/platform/win32/Crypt32Util.java index 834eb42cee..f706573823 100644 --- a/contrib/platform/src/com/sun/jna/platform/win32/Crypt32Util.java +++ b/contrib/platform/src/com/sun/jna/platform/win32/Crypt32Util.java @@ -74,7 +74,7 @@ public static byte[] cryptProtectData(byte[] data, byte[] entropy, int flags, return pDataProtected.getData(); } finally { if (pDataProtected.pbData != null) { - Kernel32Util.validateFreeLocalMemory(pDataProtected.pbData); + Kernel32Util.freeLocalMemory(pDataProtected.pbData); } } } @@ -133,25 +133,25 @@ public static byte[] cryptUnprotectData(byte[] data, byte[] entropy, int flags, } } finally { if (pDataUnprotected.pbData != null) { - int rc = Kernel32Util.freeLocalMemory(pDataUnprotected.pbData); - if (rc != WinError.ERROR_SUCCESS) { - Win32Exception exc = new Win32Exception(rc); + try { + Kernel32Util.freeLocalMemory(pDataUnprotected.pbData); + } catch(Win32Exception e) { if (err == null) { - err = exc; + err = e; } else { - err.addSuppressed(exc); + err.addSuppressed(e); } } } if (pDescription.getValue() != null) { - int rc = Kernel32Util.freeLocalMemory(pDescription.getValue()); - if (rc != WinError.ERROR_SUCCESS) { - Win32Exception exc = new Win32Exception(rc); + try { + Kernel32Util.freeLocalMemory(pDescription.getValue()); + } catch(Win32Exception e) { if (err == null) { - err = exc; + err = e; } else { - err.addSuppressed(exc); + err.addSuppressed(e); } } } diff --git a/contrib/platform/src/com/sun/jna/platform/win32/Kernel32Util.java b/contrib/platform/src/com/sun/jna/platform/win32/Kernel32Util.java index 9f23916c63..93ea3ac7b7 100644 --- a/contrib/platform/src/com/sun/jna/platform/win32/Kernel32Util.java +++ b/contrib/platform/src/com/sun/jna/platform/win32/Kernel32Util.java @@ -54,48 +54,16 @@ public static String getComputerName() { return Native.toString(buffer); } - /** - * Makes sure that local memory has been successfully freed - * - * @param ptr The {@link Pointer} to the memory to be released - ignored if NULL - * @see #freeLocalMemory(Pointer) - * @throws Win32Exception if non-{@code ERROR_SUCCESS} code reported - */ - public static void validateFreeLocalMemory(Pointer ptr) { - int rc = freeLocalMemory(ptr); - if (rc != WinError.ERROR_SUCCESS) { - throw new Win32Exception(rc); - } - } - /** * Invokes {@link Kernel32#LocalFree(Pointer)} and checks if it succeeded. * * @param ptr The {@link Pointer} to the memory to be released - ignored if NULL - * @return {@code ERROR_SUCCESS} or reason for failing to free the memory - * @see Native#getLastError() + * @throws Win32Exception if non-{@code ERROR_SUCCESS} code reported */ - public static int freeLocalMemory(Pointer ptr) { + public static void freeLocalMemory(Pointer ptr) { Pointer res = Kernel32.INSTANCE.LocalFree(ptr); if (res != null) { - return Native.getLastError(); - } else { - return WinError.ERROR_SUCCESS; - } - } - - - /** - * Makes sure that global memory has been successfully freed - * - * @param ptr The {@link Pointer} to the memory to be released - ignored if NULL - * @see #freeGlobalMemory(Pointer) - * @throws Win32Exception if non-{@code ERROR_SUCCESS} code reported - */ - public static void validateFreeGlobalMemory(Pointer ptr) { - int rc = freeGlobalMemory(ptr); - if (rc != WinError.ERROR_SUCCESS) { - throw new Win32Exception(rc); + throw new Win32Exception(Kernel32.INSTANCE.GetLastError()); } } @@ -103,15 +71,12 @@ public static void validateFreeGlobalMemory(Pointer ptr) { * Invokes {@link Kernel32#GlobalFree(Pointer)} and checks if it succeeded. * * @param ptr The {@link Pointer} to the memory to be released - ignored if NULL - * @return {@code ERROR_SUCCESS} or reason for failing to free the memory - * @see Native#getLastError() + * @throws Win32Exception if non-{@code ERROR_SUCCESS} code reported */ - public static int freeGlobalMemory(Pointer ptr) { + public static void freeGlobalMemory(Pointer ptr) { Pointer res = Kernel32.INSTANCE.GlobalFree(ptr); if (res != null) { - return Native.getLastError(); - } else { - return WinError.ERROR_SUCCESS; + throw new Win32Exception(Kernel32.INSTANCE.GetLastError()); } } @@ -141,7 +106,7 @@ public static String formatMessage(int code) { String s = ptr.getWideString(0); return s.trim(); } finally { - validateFreeLocalMemory(ptr); + freeLocalMemory(ptr); } } diff --git a/contrib/platform/test/com/sun/jna/platform/win32/Advapi32Test.java b/contrib/platform/test/com/sun/jna/platform/win32/Advapi32Test.java index 7c3fdb967e..3f6763536f 100755 --- a/contrib/platform/test/com/sun/jna/platform/win32/Advapi32Test.java +++ b/contrib/platform/test/com/sun/jna/platform/win32/Advapi32Test.java @@ -145,8 +145,7 @@ public void testIsValidSid() { assertTrue("Non positive sid length", sidLength > 0); assertTrue("Invalid sid", Advapi32.INSTANCE.IsValidSid(value)); } finally { - assertEquals("Failed to release SID", - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(value.getPointer())); + Kernel32Util.freeLocalMemory(value.getPointer()); } } @@ -159,8 +158,7 @@ public void testGetSidLength() { try { assertEquals("Wrong SID length", 12, Advapi32.INSTANCE.GetLengthSid(value)); } finally { - assertEquals("Failed to free SID", - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(value.getPointer())); + Kernel32Util.freeLocalMemory(value.getPointer()); } } @@ -194,8 +192,7 @@ public void testLookupAccountSid() { assertEquals("Everyone", nameString); assertTrue(referencedDomainNameString.length() == 0); } finally { - assertEquals("Failed to release sid", - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(value.getPointer())); + Kernel32Util.freeLocalMemory(value.getPointer()); } } @@ -214,12 +211,10 @@ public void testConvertSid() { String convertedSidString = conv.getWideString(0); assertEquals("Mismatched SID string", convertedSidString, sidString); } finally { - assertEquals("Failed to release string value", - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(conv)); + Kernel32Util.freeLocalMemory(conv); } } finally { - assertEquals("Failed to release sid", - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(value.getPointer())); + Kernel32Util.freeLocalMemory(value.getPointer()); } } @@ -631,8 +626,7 @@ public void testIsWellKnownSid() { assertTrue("Not a world sid", Advapi32.INSTANCE.IsWellKnownSid(value, WELL_KNOWN_SID_TYPE.WinWorldSid)); assertFalse("Unexpected admin sid", Advapi32.INSTANCE.IsWellKnownSid(value, WELL_KNOWN_SID_TYPE.WinAccountAdministratorSid)); } finally { - assertEquals("Failed to release sid", - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(value.getPointer())); + Kernel32Util.freeLocalMemory(value.getPointer()); } } @@ -652,8 +646,7 @@ public void testCreateWellKnownSid() { String convertedSidString = conv.getWideString(0); assertEquals("Mismatched SID string", EVERYONE, convertedSidString); } finally { - assertEquals("Failed to release string", - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(conv)); + Kernel32Util.freeLocalMemory(conv); } } @@ -1011,8 +1004,7 @@ public void testGetNamedSecurityInfoForFileNoSACL() throws Exception { file.delete(); } } finally { - assertEquals("Failed to free security descriptor of " + file, - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue())); + Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue()); } } @@ -1084,8 +1076,7 @@ public void testGetNamedSecurityInfoForFileWithSACL() throws Exception { file.delete(); } } finally { - assertEquals("Failed to free security descriptor of " + filePath, - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue())); + Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue()); } if (impersontating) { Advapi32.INSTANCE.SetThreadToken(null, null); @@ -1137,8 +1128,7 @@ public void testSetNamedSecurityInfoForFileNoSACL() throws Exception { file.delete(); } } finally { - assertEquals("Failed to release security descriptor of " + file, - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue())); + Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue()); } } @@ -1225,8 +1215,7 @@ public void testSetNamedSecurityInfoForFileWithSACL() throws Exception { file.delete(); } } finally { - assertEquals("Failed to release security descriptor of " + file, - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue())); + Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue()); } if (impersontating) { @@ -1269,8 +1258,7 @@ public void testGetSecurityDescriptorLength() throws Exception { file.delete(); } } finally { - assertEquals("Failed to release security descriptor of " + file, - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue())); + Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue()); } } @@ -1301,8 +1289,7 @@ public void testIsValidSecurityDescriptor() throws Exception { file.delete(); } } finally { - assertEquals("Failed to release security descriptor of " + file, - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue())); + Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue()); } } diff --git a/contrib/platform/test/com/sun/jna/platform/win32/Crypt32Test.java b/contrib/platform/test/com/sun/jna/platform/win32/Crypt32Test.java index ff8bd1bd3c..246aca0977 100644 --- a/contrib/platform/test/com/sun/jna/platform/win32/Crypt32Test.java +++ b/contrib/platform/test/com/sun/jna/platform/win32/Crypt32Test.java @@ -44,16 +44,13 @@ public void testCryptProtectUnprotectData() { assertEquals("description", pDescription.getValue().getWideString(0)); assertEquals("hello world", pDataDecrypted.pbData.getString(0)); } finally { - assertEquals("Failed to free decrypted data", - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(pDataDecrypted.pbData)); + Kernel32Util.freeLocalMemory(pDataDecrypted.pbData); } } finally { - assertEquals("Failed to free description", - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(pDescription.getValue())); + Kernel32Util.freeLocalMemory(pDescription.getValue()); } } finally { - assertEquals("Failed to free encrypted data", - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(pDataEncrypted.pbData)); + Kernel32Util.freeLocalMemory(pDataEncrypted.pbData); } } @@ -80,16 +77,13 @@ public void testCryptProtectUnprotectDataWithEntropy() { assertEquals("description", pDescription.getValue().getWideString(0)); assertEquals("hello world", pDataDecrypted.pbData.getString(0)); } finally { - assertEquals("Failed to free descrypted data", - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(pDataDecrypted.pbData)); + Kernel32Util.freeLocalMemory(pDataDecrypted.pbData); } } finally { - assertEquals("Failed to free description", - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(pDescription.getValue())); + Kernel32Util.freeLocalMemory(pDescription.getValue()); } } finally { - assertEquals("Failed to free encrypted data", - WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(pDataEncrypted.pbData)); + Kernel32Util.freeLocalMemory(pDataEncrypted.pbData); } } diff --git a/contrib/platform/test/com/sun/jna/platform/win32/Kernel32UtilTest.java b/contrib/platform/test/com/sun/jna/platform/win32/Kernel32UtilTest.java index a27932e8d1..d6106e449b 100644 --- a/contrib/platform/test/com/sun/jna/platform/win32/Kernel32UtilTest.java +++ b/contrib/platform/test/com/sun/jna/platform/win32/Kernel32UtilTest.java @@ -23,8 +23,10 @@ import java.util.List; import java.util.Map; +import com.sun.jna.Pointer; import com.sun.jna.platform.win32.Tlhelp32.MODULEENTRY32W; import com.sun.jna.platform.win32.WinNT.HANDLE; +import com.sun.jna.platform.win32.WinNT.HRESULT; import com.sun.jna.platform.win32.WinNT.LARGE_INTEGER; import junit.framework.TestCase; @@ -91,6 +93,30 @@ private static String formatBytes(long bytes) { } } + public void testFreeLocalMemory() { + try { + Pointer ptr = new Pointer(0xFFFFFFFFFFFFFFFFL); + Kernel32Util.freeLocalMemory(ptr); + fail("Unexpected success to free bad local memory"); + } catch(Win32Exception e) { + HRESULT hr = e.getHR(); + int code = W32Errors.HRESULT_CODE(hr.intValue()); + assertEquals("Mismatched failure reason code", WinError.ERROR_INVALID_HANDLE, code); + } + } + + public void testFreeGlobalMemory() { + try { + Pointer ptr = new Pointer(0xFFFFFFFFFFFFFFFFL); + Kernel32Util.freeGlobalMemory(ptr); + fail("Unexpected success to free bad global memory"); + } catch(Win32Exception e) { + HRESULT hr = e.getHR(); + int code = W32Errors.HRESULT_CODE(hr.intValue()); + assertEquals("Mismatched failure reason code", WinError.ERROR_INVALID_HANDLE, code); + } + } + public void testGetComputerName() { assertTrue(Kernel32Util.getComputerName().length() > 0); } diff --git a/contrib/platform/test/com/sun/jna/platform/win32/VersionTest.java b/contrib/platform/test/com/sun/jna/platform/win32/VersionTest.java index 524dd9f8d4..7d942c2a2b 100644 --- a/contrib/platform/test/com/sun/jna/platform/win32/VersionTest.java +++ b/contrib/platform/test/com/sun/jna/platform/win32/VersionTest.java @@ -54,8 +54,7 @@ public void testGetFileVersion() { assertTrue("dwFileVersionLS", fixedFileInfo.dwFileVersionLS.longValue() > 0); assertTrue("dwFileVersionMS", fixedFileInfo.dwFileVersionMS.longValue() > 0); } finally { - assertEquals("Failed to free buffer", - WinError.ERROR_SUCCESS, Kernel32Util.freeGlobalMemory(buffer)); + Kernel32Util.freeGlobalMemory(buffer); } } }