As mentioned in the comments, the function becomes much simpler by using std::vector instead of manual memory management using malloc and free.
Not only simpler, but the possibility of memory leaks becomes minimized, if not totally eliminated.
In addition, arr was not allocated using malloc, thus calling free(arr); is incorrect.
Here is your function rewritten (not tested), that totally eliminates all the calls to free():
#include <vector>
//...
struct LocalRefHandler
{
jobject *m_pObject;
JNIEnv *m_pEnv;
LocalRefHandler(JNIEnv* pEnv, jobject* instance) :
m_pEnv(pEnv), m_pObject(instance) {}
// Release the resources
~LocalRefHandler() { m_pEnv->DeleteLocalRef(*m_pObject); }
};
JNIEXPORT jint JNICALL Java_FocasUser_Focas2_getMacroDataArray
(JNIEnv *env, jobject instance, jint handle, jint startIndex, jint endIndex, jdoubleArray jArr) {
// This will automatically call DeleteLocalRef when the function exits
LocalRefHandler refHandler(env, &instance);
int len = env->GetArrayLength(jArr);
// This will clean up the memory for us when the function returns
std::vector<char> vc(8 + (8 * len));
// point to the underlying data
IODMBR* macro = static_cast<IODMBR*>(vc.data());
clock_t before = clock();
if (PrintLogCat) LOGGER.debug("cnc_rdmacror.. start Index: [%d]\t endIndex:[%d]\t",1);
short ret = cnc_rdmacror(handle, startIndex, endIndex, 8 + (8 * len), macro);
int elapsedTime = calcElapsedTime(before, clock());
loggerStream << "[" << "cnc_rdmacror" << "]=" << ret << "\tElapsed Time ms : " << elapsedTime;
fileLogger(loggerStream.str(), LOG_FILE_WRITE_COND(elapsedTime));
loggerStream.str("");
jdouble *arr = env->GetDoubleArrayElements(jArr, 0);
double temp = 0;
if (ret == EW_OK) {
for (int idx = 0; idx <= endIndex - startIndex; idx++) {
temp = macro->data[idx].mcr_val;
for (int i = 0; i < macro->data[idx].dec_val; i++) {
temp = temp / 10;
}
arr[idx] = temp;
}
} else {
env->ReleaseDoubleArrayElements(jArr, arr, 0);
if (PrintLogCat) LOGGER.error("getMacroValue pmc error : %d", ret,1);
return -1;
}
env->ReleaseDoubleArrayElements(jArr, arr, 0);
return 0;
}
The std::vector will automatically release the memory on function return, regardless of the reason or where the function returns.
The reason why this is important is that regardless of where the function returns, the std::vector will "free" the memory automatically. This includes if an exception is thrown for some reason. So not only do you not have to worry about multiple return points, even if an exception is thrown, the vector will deallocate the memory.
This method of programming, namely RAII, is essential if you're going to use JNI.
Since the possibility is somewhat high of exceptions being thrown, it is important that any resources acquired (such as allocated memory, file handles, various JNI structures, etc.) must be cleaned up on exit of the JNI function. Using objects that automatically clean themselves up on destruction is much easier to deal with than multiple try/catch blocks, where you're cleaning up things manually.
As an example, you are calling env->DeleteLocalRef(instance); in multiple places. What if an exception is thrown, and you do not get a chance for the env->DeleteLocalRef to be invoked? JNI programming is the prime example of where RAII techniques are almost a must.
Note that a LocalRefHandler is a struct that was created that when an object of this type is destroyed, the DeleteLocalRef is called. Once the refHandler variable is created from the JNI environment pointer and the instance, you can see that we no longer need to add DeleteLocalRef calls sprinkled within the code at the return points -- DeleteLocalRef will be called regardless of the reason the function exits once refHandler goes out of scope.
You could also create a similar class to handle the env->GetDoubleArrayElements and env->ReleaseDoubleArrayElements, so that you are cleaning up those resources automatically on function exit.
The bottom line is that you should build up your own codebase of these small RAII objects for JNI purposes, so that they are readily available to be used.
