Changeset 17

Timestamp:

09/12/07 11:03:54 AM (10 months ago)

Author:

aaron

Message:

• incorporated patch described below
  

From: <hidden>
To: <me>
Subject: CocoaICU issues
Date: Tue, 11 Sep 2007 10:59:24 -07

I just found CocoaICU yesterday and have started looking at it. I've found one significant issue and two minor ones.

Unicode string buffers aren't being retained properly. -[ICUPattern setTextToSearch:] points its instance variable textToSearch to the input UChar*, without copying the string. The
comment reads "retained by the NSString"; however, this isn't true. The string buffer is the result of -[NSString cStringUsingEncoding:], which returns an autoreleased buffer that
will be freed when the nearest autorelease pool goes out of scope.
The effect is that if some code created a pattern object and set a search string, then returned to its caller and later (like after another user event) tried to use the pattern, it
would probably get garbage results or crash, because the search string pointed to a freed block of memory.
Instead, you should malloc your own buffer, then call -getCharacters: on the string to copy them into that buffer.

Flags in ICUPattern.h are not "const". The types of CaseInsensitiveMatching?, etc. should be "const unsigned", not "unsigned", otherwise the values could be accidentally modified.

Names of flags should be namespaced. Those same constants have very generic names; that will cause problems if the app declares any other symbols with names like "Comments" or
"Multiline". Like your classnames, the constants should have an "ICU" prefix — "ICUCaseInsensitiveMatching", etc.

Below is a patch that fixes these. (I haven't yet had a chance to test it besides just checking that the little test app didn't crash.)

Files:

• trunk/source/ICUPattern.h (modified) (1 diff)
• trunk/source/ICUPattern.m (modified) (6 diffs)
• trunk/source/NSStringICUAdditions.h (modified) (1 diff)
• trunk/source/NSStringICUAdditions.m (modified) (1 diff)

trunk/source/ICUPattern.h

@@ -14 +14 @@
- * flags. see http://icu.sourceforge.net/apiref/icu4c/uregex_8h.html#874989dfec4cbeb6baf4d1a51cb529ae
- */
-unsigned 
-unsigned 
-unsigned 
-unsigned 
-unsigned 
+const unsigned ICU
+const unsigned ICU
+const unsigned ICU
+const unsigned ICU
+const unsigned ICU
-
-/*!

trunk/source/ICUPattern.m

@@ -29 +29 @@
-#import <unicode/ustring.h>
-
-
-
-
-
-
+const ICU
+const ICU
+const ICU
+const ICU
+const ICU
-
-@interface ICUPattern (Private)
@@ -39 +39 @@
--(unsigned)flags;
--(UChar *)textToSearch;
--(void)setTextToSearch:(UChar *)utf16String;
-
-@end
@@ -85 +84 @@
-                free(re);
-
-
+
+
-        [super dealloc];
-}
@@ -94 +94 @@
-
--(void)setStringToSearch:(NSString *)aStringToSearchOver {
-
-
-
-
-
-
-
-
-
+
+
-        UErrorCode status = 0;
-
@@ -109 +102 @@
-        [self reset];
-
-        textToSearch = utf16String; // retained by the NSString 
-        
-        if(U_FAILURE(status)) {
-textToSearch = NULL
+free(utf16String)
-                [NSException raise:@"Invalid String Exception"
-                                        format:@"Could not set text to match against: %s", u_errorName(status)];
-        }
+
+        if(textToSearch)
+                free(textToSearch);
+        textToSearch = utf16String;
-}
-
@@ -187 +182 @@
--(NSArray *)componentsSplitFromString:(NSString *)stringToSplit
-{
-TextToSearch:[stringToSplit UTF16String]
+StringToSearch:stringToSplit
-        BOOL isDone = NO;
-        UErrorCode status = 0;

trunk/source/NSStringICUAdditions.h

@@ -16 +16 @@
-+(NSString *)stringWithICUString:(void *)utf16EncodedString;
--(void *)UTF16String;
+-(void *)copyUTF16String;
-
--(NSArray *)findPattern:(NSString *)aRegex;

trunk/source/NSStringICUAdditions.m

@@ -86 +86 @@
-}
-
+-(void *)copyUTF16String {
+        unsigned int length = [self length];
+        UChar *utf16String = malloc((length+1)*sizeof(UChar));
+        [self getCharacters: utf16String];
+        utf16String[length] = 0;
+        return utf16String;
+}
+
-@end