diff --git a/src/main/cpp/_nix_based/jssc.cpp b/src/main/cpp/_nix_based/jssc.cpp index 87fb1a753..b9c87f201 100644 --- a/src/main/cpp/_nix_based/jssc.cpp +++ b/src/main/cpp/_nix_based/jssc.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -657,10 +658,27 @@ JNIEXPORT jbyteArray JNICALL Java_jssc_SerialNativeInterface_readBytes (JNIEnv *env, jobject, jlong portHandle, jint byteCount){ int err; - jbyte *lpBuffer = new jbyte[byteCount]; + jbyte *lpBuffer = NULL; jbyteArray returnArray = NULL; int byteRemains = byteCount; + if( byteCount <= 0 ){ + char emsg[64]; emsg[0] = '\0'; + snprintf(emsg, sizeof emsg, "byteCount %d. Expected range: 1..2147483647", byteCount); + jclass exClz = env->FindClass("java/lang/IllegalArgumentException"); + if( exClz ) env->ThrowNew(exClz, emsg); + returnArray = NULL; goto Finally; + } + + lpBuffer = (jbyte*)malloc(byteCount*sizeof*lpBuffer); + if( !lpBuffer ){ + char emsg[32]; emsg[0] = '\0'; + snprintf(emsg, sizeof emsg, "malloc(%d) failed", byteCount*sizeof*lpBuffer); + jclass exClz = env->FindClass("java/lang/RuntimeException"); + if( exClz ) env->ThrowNew(exClz, emsg); + returnArray = NULL; goto Finally; + } + while(byteRemains > 0) { int result = 0; @@ -707,7 +725,7 @@ JNIEXPORT jbyteArray JNICALL Java_jssc_SerialNativeInterface_readBytes assert(env->ExceptionCheck() == JNI_FALSE); Finally: - delete[] lpBuffer; + if( lpBuffer ) free(lpBuffer); return returnArray; } diff --git a/src/main/cpp/windows/jssc.cpp b/src/main/cpp/windows/jssc.cpp index 6c9f298b0..7c1c61bf9 100644 --- a/src/main/cpp/windows/jssc.cpp +++ b/src/main/cpp/windows/jssc.cpp @@ -276,17 +276,31 @@ JNIEXPORT jbyteArray JNICALL Java_jssc_SerialNativeInterface_readBytes HANDLE hComm = (HANDLE)portHandle; DWORD lpNumberOfBytesTransferred; DWORD lpNumberOfBytesRead; + jbyteArray returnArray = NULL; jbyte *lpBuffer = NULL; + OVERLAPPED *overlapped = NULL; - jbyteArray returnArray = env->NewByteArray(byteCount); + if( byteCount <= 0 ){ + char emsg[64]; emsg[0] = '\0'; + snprintf(emsg, sizeof emsg, "byteCount %d. Expected range: 1..2147483647", byteCount); + jclass exClz = env->FindClass("java/lang/IllegalArgumentException"); + if( exClz ) env->ThrowNew(exClz, emsg); + returnArray = NULL; goto Finally; + } - lpBuffer = (jbyte *)malloc(byteCount * sizeof(jbyte)); - if(lpBuffer == NULL){ - // return an empty array - return returnArray; + returnArray = env->NewByteArray(byteCount); + if( returnArray == NULL ) goto Finally; + + lpBuffer = (jbyte*)malloc(byteCount*sizeof*lpBuffer); + if( !lpBuffer ){ + char emsg[32]; emsg[0] = '\0'; + snprintf(emsg, sizeof emsg, "malloc(%d) failed", byteCount*sizeof*lpBuffer); + jclass exClz = env->FindClass("java/lang/RuntimeException"); + if( exClz ) env->ThrowNew(exClz, emsg); + returnArray = NULL; goto Finally; } - OVERLAPPED *overlapped = new OVERLAPPED(); + overlapped = new OVERLAPPED(); overlapped->hEvent = CreateEventA(NULL, true, false, NULL); if(ReadFile(hComm, lpBuffer, (DWORD)byteCount, &lpNumberOfBytesRead, overlapped)){ env->SetByteArrayRegion(returnArray, 0, byteCount, lpBuffer); @@ -302,9 +316,12 @@ JNIEXPORT jbyteArray JNICALL Java_jssc_SerialNativeInterface_readBytes jclass exClz = env->FindClass("java/lang/IllegalArgumentException"); if( exClz != NULL ) env->ThrowNew(exClz, "EBADF"); } - CloseHandle(overlapped->hEvent); - delete overlapped; - free(lpBuffer); +Finally: + if( overlapped ){ + CloseHandle(overlapped->hEvent); + delete overlapped; + } + if( lpBuffer ) free(lpBuffer); return returnArray; } diff --git a/src/test/java/jssc/SerialNativeInterfaceTest.java b/src/test/java/jssc/SerialNativeInterfaceTest.java index fc4f94d2e..a1186c923 100644 --- a/src/test/java/jssc/SerialNativeInterfaceTest.java +++ b/src/test/java/jssc/SerialNativeInterfaceTest.java @@ -4,6 +4,7 @@ import org.junit.Assume; import org.junit.Before; import org.junit.Test; +import org.slf4j.Logger; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; @@ -13,9 +14,12 @@ import static org.junit.Assert.fail; import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; +import static org.slf4j.LoggerFactory.getLogger;; public class SerialNativeInterfaceTest extends DisplayMethodNameRule { + private static final Logger log = getLogger(SerialNativeInterfaceTest.class); + @Test public void testInitNativeInterface() { SerialNativeInterface serial = new SerialNativeInterface(); @@ -94,4 +98,48 @@ public void throwsNpeIfPassedBufferIsNull() throws Exception { new SerialNativeInterface().writeBytes(1, null); } + @Test + public void throwsIfCountNegative() throws Exception { + SerialNativeInterface testTarget = new SerialNativeInterface(); + byte[] ret; + try{ + ret = testTarget.readBytes(0, -42); + fail("Where's the exception?"); + }catch( IllegalArgumentException ex ){ + assertTrue(ex.getMessage().contains("-42")); + } + } + + @Test + public void throwsIfCountZero() throws Exception { + SerialNativeInterface testTarget = new SerialNativeInterface(); + byte[] ret; + try{ + ret = testTarget.readBytes(0, 0); + fail("Where's the exception?"); + }catch( IllegalArgumentException ex ){ + assertTrue(ex.getMessage().contains("0")); + } + } + + @Test + @org.junit.Ignore("This test only makes sense if it is run in a situation" + +" where large memory allocations WILL fail (for example you could use" + +" a virtual machine with low memory available). Because on regular" + +" machines allocating 2GiB of RAM is not a problem at all and so the" + +" test run will just happily wait infinitely for those 2GiB to arrive" + +" at the stdin fd. Feel free to remove this test if you think it" + +" doesn't make sense to have it here.") + public void throwsIfRequestTooLarge() throws Exception { + SerialNativeInterface testTarget = new SerialNativeInterface(); + int tooLargeSize = Integer.MAX_VALUE; + try{ + byte[] ret = testTarget.readBytes(0, tooLargeSize); + fail("Where's the exception?"); + }catch( RuntimeException ex ){ + log.debug("Thrown, as expected :)", ex); + assertTrue(ex.getMessage().contains(String.valueOf(tooLargeSize))); + } + } + } diff --git a/src/test/resources/log4j2.properties b/src/test/resources/log4j2.properties index ce58c24b1..bb6d07233 100644 --- a/src/test/resources/log4j2.properties +++ b/src/test/resources/log4j2.properties @@ -1,7 +1,7 @@ # Logging settings for unit tests # The root logger with appender name -rootLogger=DEBUG,STDOUT,TESTNAME +rootLogger=INFO,STDOUT,TESTNAME # Assign STDOUT a valid appender & define its layout appender.console.name=STDOUT @@ -19,4 +19,8 @@ appender.testName.filter.1.marker=MethodName appender.console.filter.1.type=MarkerFilter appender.console.filter.1.marker=MethodName appender.console.filter.1.onMatch=DENY -appender.console.filter.1.onMismatch=ACCEPT \ No newline at end of file +appender.console.filter.1.onMismatch=ACCEPT + +# Configures log-level of 'jssc' java package (Handy for debugging tests). +#logger.jssc.name = jssc +#logger.jssc.level = DEBUG