Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kernel32Util#freeLocal/GlobalMemory always throws Win32Exception if failed #612

Merged
merged 1 commit into from
Mar 6, 2016
Merged

Kernel32Util#freeLocal/GlobalMemory always throws Win32Exception if failed #612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
---------
Expand Down
8 changes: 4 additions & 4 deletions contrib/platform/src/com/sun/jna/platform/win32/Advapi32Util.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ public static String convertSidToStringSid(PSID sid) {
try {
return ptr.getWideString(0);
} finally {
Kernel32Util.validateFreeLocalMemory(ptr);
Kernel32Util.freeLocalMemory(ptr);
}
}

Expand All @@ -323,7 +323,7 @@ public static byte[] convertStringSidToSid(String sidString) {
try {
return value.getBytes();
} finally {
Kernel32Util.validateFreeLocalMemory(value.getPointer());
Kernel32Util.freeLocalMemory(value.getPointer());
}
}

Expand All @@ -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());
}
}

Expand Down Expand Up @@ -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);
}
}

Expand Down
22 changes: 11 additions & 11 deletions contrib/platform/src/com/sun/jna/platform/win32/Crypt32Util.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
49 changes: 7 additions & 42 deletions contrib/platform/src/com/sun/jna/platform/win32/Kernel32Util.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,64 +54,29 @@ 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());
}
}

/**
* 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());
}
}

Expand Down Expand Up @@ -141,7 +106,7 @@ public static String formatMessage(int code) {
String s = ptr.getWideString(0);
return s.trim();
} finally {
validateFreeLocalMemory(ptr);
freeLocalMemory(ptr);
}
}

Expand Down
39 changes: 13 additions & 26 deletions contrib/platform/test/com/sun/jna/platform/win32/Advapi32Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}

Expand All @@ -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());
}
}

Expand Down Expand Up @@ -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());
}
}

Expand All @@ -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());
}
}

Expand Down Expand Up @@ -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());
}
}

Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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());
}
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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());
}
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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());
}
}

Expand Down Expand Up @@ -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());
}
}

Expand Down
18 changes: 6 additions & 12 deletions contrib/platform/test/com/sun/jna/platform/win32/Crypt32Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand All @@ -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);
}
}

Expand Down
26 changes: 26 additions & 0 deletions contrib/platform/test/com/sun/jna/platform/win32/Kernel32UtilTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
3 changes: 1 addition & 2 deletions contrib/platform/test/com/sun/jna/platform/win32/VersionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}