]> granicus.if.org Git - icu/commitdiff
ICU-11711 better memory management in GenrbImporter::getRules() using string class...
authorMarkus Scherer <markus.icu@gmail.com>
Tue, 16 Jun 2015 11:23:04 +0000 (11:23 +0000)
committerMarkus Scherer <markus.icu@gmail.com>
Tue, 16 Jun 2015 11:23:04 +0000 (11:23 +0000)
X-SVN-Rev: 37575

icu4c/source/test/intltest/collationtest.cpp
icu4c/source/test/intltest/datadrivennumberformattestsuite.cpp
icu4c/source/tools/genrb/parse.cpp
icu4c/source/tools/toolutil/ucbuf.h

index b0202d193ed6bf1b6fec40580407373042dc6543..1248e1ce44335a1ecc3e4d32c721873caba63bd6 100644 (file)
@@ -48,9 +48,6 @@
 #include "uvectr64.h"
 #include "writesrc.h"
 
-// TODO: Move to ucbuf.h
-U_DEFINE_LOCAL_OPEN_POINTER(LocalUCHARBUFPointer, UCHARBUF, ucbuf_close);
-
 class CodePointIterator;
 
 // TODO: try to share code with IntlTestCollator; for example, prettify(CollationKey)
index 3fd7023deba9bfde0e63518e0289d898334418af..8c4623af595b370c1fe2f2db23e91288822bff7f 100644 (file)
@@ -10,8 +10,6 @@
 #include "unicode/localpointer.h"
 #include "ustrfmt.h"
 
-U_DEFINE_LOCAL_OPEN_POINTER(LocalUCHARBUFPointer, UCHARBUF, ucbuf_close);
-
 static UBool isCROrLF(UChar c) { return c == 0xa || c == 0xd; }
 static UBool isSpace(UChar c) { return c == 9 || c == 0x20 || c == 0x3000; }
 
index 95386dea5168b6fcc98e026c802dcc2bef1985ec..c0ef6bc3bbc2980099f9964f86bce39d9aa048f9 100644 (file)
 #include "reslist.h"
 #include "rbt_pars.h"
 #include "genrb.h"
+#include "unicode/stringpiece.h"
 #include "unicode/ustring.h"
 #include "unicode/uscript.h"
 #include "unicode/utf16.h"
 #include "unicode/putil.h"
+#include "charstr.h"
 #include "collationbuilder.h"
 #include "collationdata.h"
 #include "collationdatareader.h"
 #define OPENSQBRACKET    0x005B
 #define CLOSESQBRACKET   0x005D
 
+using icu::CharString;
 using icu::LocalPointer;
+using icu::LocalUCHARBUFPointer;
+using icu::StringPiece;
 using icu::UnicodeString;
 
 struct Lookahead
@@ -690,47 +695,20 @@ GenrbImporter::getRules(
         const char *localeID, const char *collationType,
         UnicodeString &rules,
         const char *& /*errorReason*/, UErrorCode &errorCode) {
-    struct SRBRoot *data         = NULL;
-    UCHARBUF       *ucbuf        = NULL;
-    int localeLength = strlen(localeID);
-    char* filename = (char*)uprv_malloc(localeLength+5);
-    char           *inputDirBuf  = NULL;
-    char           *openFileName = NULL;
-    const char* cp = "";
-    int32_t i = 0;
-    int32_t dirlen  = 0;
-    int32_t filelen = 0;
-    struct SResource* root;
-    struct SResource* collations;
-    struct SResource* collation;
-    struct SResource* sequence;
-
-    memcpy(filename, localeID, localeLength);
-    for(i = 0; i < localeLength; i++){
+    CharString filename(localeID, errorCode);
+    for(int32_t i = 0; i < filename.length(); i++){
         if(filename[i] == '-'){
-            filename[i] = '_';
+            filename.data()[i] = '_';
         }
     }
-    filename[localeLength]   = '.';
-    filename[localeLength+1] = 't';
-    filename[localeLength+2] = 'x';
-    filename[localeLength+3] = 't';
-    filename[localeLength+4] = 0;
-
-
+    filename.append(".txt", errorCode);
     if (U_FAILURE(errorCode)) {
         return;
     }
-    if(filename==NULL){
-        errorCode=U_ILLEGAL_ARGUMENT_ERROR;
-        return;
-    }else{
-        filelen = (int32_t)uprv_strlen(filename);
-    }
+    CharString inputDirBuf;
+    CharString openFileName;
     if(inputDir == NULL) {
-        const char *filenameBegin = uprv_strrchr(filename, U_FILE_SEP_CHAR);
-        openFileName = (char *) uprv_malloc(dirlen + filelen + 2);
-        openFileName[0] = '\0';
+        const char *filenameBegin = uprv_strrchr(filename.data(), U_FILE_SEP_CHAR);
         if (filenameBegin != NULL) {
             /*
              * When a filename ../../../data/root.txt is specified,
@@ -738,36 +716,19 @@ GenrbImporter::getRules(
              * This is very important when the resource file includes
              * another file, like UCARules.txt or thaidict.brk.
              */
-            int32_t filenameSize = (int32_t)(filenameBegin - filename + 1);
-            inputDirBuf = (char *)uprv_malloc(filenameSize);
-
-            /* test for NULL */
-            if(inputDirBuf == NULL) {
-                errorCode = U_MEMORY_ALLOCATION_ERROR;
-                goto finish;
-            }
-
-            uprv_strncpy(inputDirBuf, filename, filenameSize);
-            inputDirBuf[filenameSize - 1] = 0;
-            inputDir = inputDirBuf;
-            dirlen  = (int32_t)uprv_strlen(inputDir);
+            StringPiece dir = filename.toStringPiece();
+            const char *filenameLimit = filename.data() + filename.length();
+            dir.remove_suffix((int32_t)(filenameLimit - filenameBegin));
+            inputDirBuf.append(dir, errorCode);
+            inputDir = inputDirBuf.data();
         }
     }else{
-        dirlen  = (int32_t)uprv_strlen(inputDir);
-
-        if(inputDir[dirlen-1] != U_FILE_SEP_CHAR) {
-            openFileName = (char *) uprv_malloc(dirlen + filelen + 2);
+        int32_t dirlen  = (int32_t)uprv_strlen(inputDir);
 
-            /* test for NULL */
-            if(openFileName == NULL) {
-                errorCode = U_MEMORY_ALLOCATION_ERROR;
-                goto finish;
-            }
-
-            openFileName[0] = '\0';
+        if((filename[0] != U_FILE_SEP_CHAR) && (inputDir[dirlen-1] !='.')) {
             /*
              * append the input dir to openFileName if the first char in
-             * filename is not file seperation char and the last char input directory is  not '.'.
+             * filename is not file separator char and the last char input directory is  not '.'.
              * This is to support :
              * genrb -s. /home/icu/data
              * genrb -s. icu/data
@@ -776,70 +737,48 @@ GenrbImporter::getRules(
              * user should use
              * genrb -s. icu/data  --- start from CWD and look in icu/data dir
              */
-            if( (filename[0] != U_FILE_SEP_CHAR) && (inputDir[dirlen-1] !='.')){
-                uprv_strcpy(openFileName, inputDir);
-                openFileName[dirlen]     = U_FILE_SEP_CHAR;
+            openFileName.append(inputDir, dirlen, errorCode);
+            if(inputDir[dirlen-1] != U_FILE_SEP_CHAR) {
+                openFileName.append(U_FILE_SEP_CHAR, errorCode);
             }
-            openFileName[dirlen + 1] = '\0';
-        } else {
-            openFileName = (char *) uprv_malloc(dirlen + filelen + 1);
-
-            /* test for NULL */
-            if(openFileName == NULL) {
-                errorCode = U_MEMORY_ALLOCATION_ERROR;
-                goto finish;
-            }
-
-            uprv_strcpy(openFileName, inputDir);
-
         }
     }
-    uprv_strcat(openFileName, filename);
-    /* printf("%s\n", openFileName);  */
-    errorCode = U_ZERO_ERROR;
-    ucbuf = ucbuf_open(openFileName, &cp,getShowWarning(),TRUE, &errorCode);
-
+    openFileName.append(filename, errorCode);
+    if(U_FAILURE(errorCode)) {
+        return;
+    }
+    // printf("GenrbImporter::getRules(%s, %s) reads %s\n", localeID, collationType, openFileName.data());
+    const char* cp = "";
+    LocalUCHARBUFPointer ucbuf(
+            ucbuf_open(openFileName.data(), &cp, getShowWarning(), TRUE, &errorCode));
     if(errorCode == U_FILE_ACCESS_ERROR) {
-
-        fprintf(stderr, "couldn't open file %s\n", openFileName == NULL ? filename : openFileName);
-        goto finish;
+        fprintf(stderr, "couldn't open file %s\n", openFileName.data());
+        return;
     }
-    if (ucbuf == NULL || U_FAILURE(errorCode)) {
-        fprintf(stderr, "An error occured processing file %s. Error: %s\n", openFileName == NULL ? filename : openFileName,u_errorName(errorCode));
-        goto finish;
+    if (ucbuf.isNull() || U_FAILURE(errorCode)) {
+        fprintf(stderr, "An error occured processing file %s. Error: %s\n", openFileName.data(), u_errorName(errorCode));
+        return;
     }
 
     /* Parse the data into an SRBRoot */
-    data = parse(ucbuf, inputDir, outputDir, filename, FALSE, FALSE, &errorCode);
+    struct SRBRoot *data =
+            parse(ucbuf.getAlias(), inputDir, outputDir, filename.data(), FALSE, FALSE, &errorCode);
     if (U_FAILURE(errorCode)) {
-        goto finish;
+        return;
     }
 
-    root = data->fRoot;
-    collations = resLookup(root, "collations");
+    struct SResource *root = data->fRoot;
+    struct SResource *collations = resLookup(root, "collations");
     if (collations != NULL) {
-      collation = resLookup(collations, collationType);
+      struct SResource *collation = resLookup(collations, collationType);
       if (collation != NULL) {
-        sequence = resLookup(collation, "Sequence");
+        struct SResource *sequence = resLookup(collation, "Sequence");
         if (sequence != NULL) {
           // No string pointer aliasing so that we need not hold onto the resource bundle.
           rules.setTo(sequence->u.fString.fChars, sequence->u.fString.fLength);
         }
       }
     }
-
-finish:
-    if (inputDirBuf != NULL) {
-        uprv_free(inputDirBuf);
-    }
-
-    if (openFileName != NULL) {
-        uprv_free(openFileName);
-    }
-
-    if(ucbuf) {
-        ucbuf_close(ucbuf);
-    }
 }
 
 // Quick-and-dirty escaping function.
@@ -1027,7 +966,7 @@ addCollation(ParseState* state, struct SResource  *result, const char *collation
             escape(parseError.postContext, postBuffer);
             error(line, "  error context: \"...%s\" ! \"%s...\"", preBuffer, postBuffer);
         }
-        if(isStrict()) {
+        if(isStrict() || t.isNull()) {
             *status = intStatus;
             res_close(result);
             return NULL;
index 100e29b7f9bd4383c588e3a3f66aed5f4c244f60..37fc783dec834802c3fbd6cba30016b583caf152 100644 (file)
@@ -1,7 +1,7 @@
 /*
 *******************************************************************************
 *
-*   Copyright (C) 1998-2008, International Business Machines
+*   Copyright (C) 1998-2015, International Business Machines
 *   Corporation and others.  All Rights Reserved.
 *
 *******************************************************************************
@@ -137,6 +137,16 @@ ucbuf_getBuffer(UCHARBUF* buf,int32_t* len,UErrorCode* err);
 U_CAPI void U_EXPORT2
 ucbuf_close(UCHARBUF* buf);
 
+#if U_SHOW_CPLUSPLUS_API
+
+U_NAMESPACE_BEGIN
+
+U_DEFINE_LOCAL_OPEN_POINTER(LocalUCHARBUFPointer, UCHARBUF, ucbuf_close);
+
+U_NAMESPACE_END
+
+#endif
+
 /**
  * Rewinds the buffer by one codepoint. Does not rewind over escaped characters.
  */