Use icu4jni_error more consistently.

In BidiWrapperInterface.c, replace check_fail with icu4jni_error for better
error reporting.

In CollationInterface.c, make sure we call icu4jni_error *after* calling
the function that may or may not return an error.

2 files changed, 79 insertions, 110 deletions

diff --git a/icu/src/main/native/BidiWrapperInterface.c b/icu/src/main/native/BidiWrapperInterface.c
index 55be7a0..19ac520 100644
--- a/icu/src/main/native/BidiWrapperInterface.c
+++ b/icu/src/main/native/BidiWrapperInterface.c
@@ -15,18 +15,17 @@
  * limitations under the License.
  */
 
+#include "BidiWrapperInterface.h"
+#include "ErrorCode.h"
+#include "unicode/ubidi.h"
 #include <stdlib.h>
-#include <unicode/ubidi.h>
 #include <string.h>
-#include "BidiWrapperInterface.h"
 
 typedef struct {
     UBiDi *pBiDi;
     void *embeddingLevels;
 } BiDiData;
 
-void check_fail (JNIEnv * env, int err);
-
 JNIEXPORT jlong JNICALL Java_org_apache_harmony_text_BidiWrapper_ubidi_1open
   (JNIEnv * env, jclass clazz)
 {
@@ -68,27 +67,27 @@ JNIEXPORT void JNICALL Java_org_apache_harmony_text_BidiWrapper_ubidi_1setPara
   ubidi_setPara ((*data).pBiDi, _text, length, paraLevel,
                  ((*data).embeddingLevels), &err);
   (*env)->ReleaseCharArrayElements (env, text, _text, 0);
-  check_fail (env, err);
-
   free(oldEmbeddingLevels);
+
+  icu4jni_error(env, err);
 }
 
 JNIEXPORT jlong JNICALL Java_org_apache_harmony_text_BidiWrapper_ubidi_1setLine
   (JNIEnv * env, jclass clazz, jlong pBiDi, jint start, jint limit)
 {
-  UErrorCode err = 0;
-  BiDiData *data = (BiDiData *)pBiDi;
-  BiDiData *lineData = (BiDiData *) malloc(sizeof(BiDiData));
-  (*lineData).embeddingLevels = NULL;
-
-  (*lineData).pBiDi = ubidi_openSized (limit - start, 0, &err);
-  check_fail (env, err);
-
-  ubidi_setLine ((*data).pBiDi, start, limit, (*lineData).pBiDi,
-                 &err);
-  check_fail (env, err);
+    UErrorCode err = 0;
+    BiDiData *data = (BiDiData *)pBiDi;
+    BiDiData *lineData = (BiDiData *) malloc(sizeof(BiDiData));
+    (*lineData).embeddingLevels = NULL;
+
+    (*lineData).pBiDi = ubidi_openSized (limit - start, 0, &err);
+    if (icu4jni_error(env, err)) {
+        return 0;
+    }
 
-  return (jlong) lineData;
+    ubidi_setLine ((*data).pBiDi, start, limit, (*lineData).pBiDi, &err);
+    icu4jni_error(env, err);
+    return (jlong) lineData;
 }
 
 JNIEXPORT jint JNICALL Java_org_apache_harmony_text_BidiWrapper_ubidi_1getDirection
@@ -115,76 +114,57 @@ JNIEXPORT jbyte JNICALL Java_org_apache_harmony_text_BidiWrapper_ubidi_1getParaL
 JNIEXPORT jbyteArray JNICALL Java_org_apache_harmony_text_BidiWrapper_ubidi_1getLevels
   (JNIEnv * env, jclass clazz, jlong pBiDi)
 {
-  UErrorCode err = 0;
-  const UBiDiLevel *levels = NULL;
-  jbyteArray result = NULL;
-  int len = 0;
-  BiDiData *data = (BiDiData *)pBiDi;
+    BiDiData *data = (BiDiData *)pBiDi;
 
-  levels = ubidi_getLevels ((*data).pBiDi, &err);
-  check_fail (env, err);
+    UErrorCode err = 0;
+    const UBiDiLevel* levels = ubidi_getLevels((*data).pBiDi, &err);
+    if (icu4jni_error(env, err)) {
+        return NULL;
+    }
 
-  len = ubidi_getLength ((*data).pBiDi);
-  result = (*env)->NewByteArray (env, len);
-  (*env)->SetByteArrayRegion (env, result, 0, len, (jbyte *) levels);
+    int len = ubidi_getLength((*data).pBiDi);
+    jbyteArray result = (*env)->NewByteArray(env, len);
+    (*env)->SetByteArrayRegion(env, result, 0, len, (jbyte*) levels);
 
-  return result;
+    return result;
 }
 
 JNIEXPORT jint JNICALL Java_org_apache_harmony_text_BidiWrapper_ubidi_1countRuns
   (JNIEnv * env, jclass clazz, jlong pBiDi)
 {
-  UErrorCode err = 0;
-  BiDiData *data = (BiDiData *)pBiDi;
-
-  int count = ubidi_countRuns ((*data).pBiDi, &err);
-  check_fail (env, err);
+    BiDiData *data = (BiDiData *)pBiDi;
 
-  return count;
+    UErrorCode err = 0;
+    int count = ubidi_countRuns ((*data).pBiDi, &err);
+    icu4jni_error(env, err);
+    return count;
 }
 
 JNIEXPORT jobjectArray JNICALL Java_org_apache_harmony_text_BidiWrapper_ubidi_1getRuns
   (JNIEnv * env, jclass clz, jlong pBiDi)
 {
-  int runCount = 0;
-  int start = 0;
-  int limit = 0;
-  int i = 0;
-  UBiDiLevel level = 0;
-  jclass run_clazz = 0;
-  jmethodID initID = 0;
-  jobject run = 0;
-  jobjectArray runs;
-  UErrorCode err = 0;
-  BiDiData *data = (BiDiData *)pBiDi;
-
-  run_clazz = (*env)->FindClass (env, "org/apache/harmony/text/BidiRun");
-  initID = (*env)->GetMethodID (env, run_clazz, "<init>", "(III)V");
+    BiDiData* data = (BiDiData*) pBiDi;
 
-  runCount = ubidi_countRuns ((*data).pBiDi, &err);
-  check_fail (env, err);
-  
-  runs = (*env)->NewObjectArray(env, runCount,run_clazz, NULL);  
-  for (i = 0; i < runCount; i++) {
-      ubidi_getLogicalRun((*data).pBiDi, start, &limit, &level);
-      run = (*env)->NewObject (env, run_clazz, initID, start, limit, level);
-      (*env)->SetObjectArrayElement(env, runs, i, run);
-      start = limit;
-  }
-  return runs;
-}
-
-void
-check_fail (JNIEnv * env, int err)
-{
-  char message[] = "ICU Internal Error:                     ";
+    UErrorCode err = 0;
+    int runCount = ubidi_countRuns((*data).pBiDi, &err);
+    if (icu4jni_error(env, err)) {
+        return 0;
+    }
 
-  if (U_FAILURE (err))
-    {
-      sprintf (message, "ICU Internal Error: %d", err);
-      jniThrowException(env, "java/lang/RuntimeException",
-                              message);
+    jclass bidiRunClass = (*env)->FindClass(env, "org/apache/harmony/text/BidiRun");
+    jmethodID bidiRunConstructor = (*env)->GetMethodID(env, bidiRunClass, "<init>", "(III)V");
+    jobjectArray runs = (*env)->NewObjectArray(env, runCount, bidiRunClass, NULL);
+    UBiDiLevel level = 0;
+    int start = 0;
+    int limit = 0;
+    int i = 0; // TODO: switch this file to C++!
+    for (i = 0; i < runCount; ++i) {
+        ubidi_getLogicalRun((*data).pBiDi, start, &limit, &level);
+        jobject run = (*env)->NewObject(env, bidiRunClass, bidiRunConstructor, start, limit, level);
+        (*env)->SetObjectArrayElement(env, runs, i, run);
+        start = limit;
     }
+    return runs;
 }
 
 JNIEXPORT jintArray JNICALL Java_org_apache_harmony_text_BidiWrapper_ubidi_1reorderVisual
diff --git a/icu/src/main/native/CollationInterface.c b/icu/src/main/native/CollationInterface.c
index 86246ac..c669649 100644
--- a/icu/src/main/native/CollationInterface.c
+++ b/icu/src/main/native/CollationInterface.c
@@ -172,13 +172,11 @@ static jint getMaxExpansion(JNIEnv *env, jclass obj,
 static jint getNormalization(JNIEnv *env, jclass obj,
         jint address) {
 
-  UErrorCode status = U_ZERO_ERROR;
-  const UCollator *collator = (const UCollator *)(int)address;
-  if(U_FAILURE(status)){
-       icu4jni_error(env, status);
-  }
-  return (jint)ucol_getAttribute(collator,UCOL_NORMALIZATION_MODE,&status);
-
+    const UCollator* collator = (const UCollator*) address;
+    UErrorCode status = U_ZERO_ERROR;
+    jint result = ucol_getAttribute(collator, UCOL_NORMALIZATION_MODE, &status);
+    icu4jni_error(env, status);
+    return result;
 }
 
 /**
@@ -192,12 +190,10 @@ static jint getNormalization(JNIEnv *env, jclass obj,
 static void setNormalization(JNIEnv *env, jclass obj, jint address, 
         jint mode) {
 
+    const UCollator* collator = (const UCollator*) address;
     UErrorCode status = U_ZERO_ERROR;
-    const UCollator *collator = (const UCollator *)(int)address;
-    if(U_FAILURE(status)){
-        icu4jni_error(env, status);
-    }
-    ucol_setAttribute(collator,UCOL_NORMALIZATION_MODE,mode,&status);
+    ucol_setAttribute(collator, UCOL_NORMALIZATION_MODE, mode, &status);
+    icu4jni_error(env, status);
 }
 
 
@@ -324,12 +320,11 @@ static jint hashCode(JNIEnv *env, jclass obj, jint address) {
 *         error has occured or if the end of string has been reached
 */
 static jint next(JNIEnv *env, jclass obj, jint address) {
-  UCollationElements *iterator = (UCollationElements *)(int)address;
-  UErrorCode status = U_ZERO_ERROR;
-  jint result = ucol_next(iterator, &status);
-
-   icu4jni_error(env, status);
-  return result;
+    UCollationElements *iterator = (UCollationElements *) address;
+    UErrorCode status = U_ZERO_ERROR;
+    jint result = ucol_next(iterator, &status);
+    icu4jni_error(env, status);
+    return result;
 }
 
 /**
@@ -343,14 +338,10 @@ static jint next(JNIEnv *env, jclass obj, jint address) {
 * @exception thrown if creation of the UCollator fails
 */
 static jint openCollator__(JNIEnv *env, jclass obj) {
-  jint result;
-  UErrorCode status = U_ZERO_ERROR;
-
-  result = (jint)ucol_open(NULL, &status);
-  if ( icu4jni_error(env, status) != FALSE)
-    return 0;
- 
-  return result;
+    UErrorCode status = U_ZERO_ERROR;
+    jint result = ucol_open(NULL, &status);
+    icu4jni_error(env, status);
+    return result;
 }
 
 
@@ -368,19 +359,18 @@ static jint openCollator__(JNIEnv *env, jclass obj) {
 static jint openCollator__Ljava_lang_String_2(JNIEnv *env,
         jclass obj, jstring locale) {
 
-  /* this will be null terminated */
-  const char *localestr = (*env)->GetStringUTFChars(env, locale, 0);
-  jint result=0;
-  UErrorCode status = U_ZERO_ERROR;
+    /* this will be null terminated */
+    const char* localeStr = (*env)->GetStringUTFChars(env, locale, NULL);
+    if (localeStr == NULL) {
+        icu4jni_error(env, U_ILLEGAL_ARGUMENT_ERROR);
+        return 0;
+    }
 
-  if(localestr){
-      result = (jint)ucol_open(localestr, &status);
-      (*env)->ReleaseStringUTFChars(env, locale, localestr);
-      icu4jni_error(env, status);
-  }else{
-      icu4jni_error(env,U_ILLEGAL_ARGUMENT_ERROR);
-  }
-  return result;
+    UErrorCode status = U_ZERO_ERROR;
+    jint result = ucol_open(localeStr, &status);
+    (*env)->ReleaseStringUTFChars(env, locale, localeStr);
+    icu4jni_error(env, status);
+    return result;
 }
 
 /**
@@ -586,4 +576,3 @@ int register_com_ibm_icu4jni_text_NativeCollator(JNIEnv *_env) {
     return jniRegisterNativeMethods(_env, "com/ibm/icu4jni/text/NativeCollation",
                 gMethods, NELEM(gMethods));
 }
-