Fixed up some CFErrorRef leaks.

Updated error logging to better reflect the type of each message.
l4u · Jul 3, 2012 · c18271e · c18271e
1 parent 84f57a3
commit c18271e
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 49 deletions.
diff --git a/RHAddressBook/RHAddressBook.h b/RHAddressBook/RHAddressBook.h
@@ -165,3 +165,5 @@ extern NSString * const RHAddressBookPersonAddressGeocodeCompleted;
 #define RHLog(format, ...)
 #endif
 
+#define RHErrorLog(format, ...) NSLog( @"%s:%i %@ ", __PRETTY_FUNCTION__, __LINE__, [NSString stringWithFormat: format, ##__VA_ARGS__])
+
diff --git a/RHAddressBook/RHAddressBook.m b/RHAddressBook/RHAddressBook.m
@@ -130,14 +130,15 @@ -(id)init{
 
 #if __IPHONE_OS_VERSION_MAX_ALLOWED >= 60000
         if (ABAddressBookCreateWithOptions != NULL){
-            __block CFErrorRef error = NULL;
+            __block CFErrorRef errorRef = NULL;
             [_addressBookThread performBlock:^{
-                _addressBookRef = ABAddressBookCreateWithOptions(nil, &error);
+                _addressBookRef = ABAddressBookCreateWithOptions(nil, &errorRef);
             }];
 
             if (!_addressBookRef){
                 //bail
-                NSLog(@"Error: Failed to create RHAddressBook instance. Underlying ABAddressBookCreateWithOptions() failed with error: %@", error);
+                RHErrorLog(@"Error: Failed to create RHAddressBook instance. Underlying ABAddressBookCreateWithOptions() failed with error: %@", errorRef);
+                if (errorRef) CFRelease(errorRef);
                 [self release];
                 return nil;
             }
@@ -151,7 +152,7 @@ -(id)init{
 
 #if __IPHONE_OS_VERSION_MAX_ALLOWED >= 60000
         }
-#endif //end iOS5+
+#endif //end iOS6+
 
         [_addressBookThread performBlock:^{
             //weak linking mutable sets
@@ -604,7 +605,8 @@ -(BOOL)addPerson:(RHPerson *)person{
         CFErrorRef errorRef = NULL;
         result = ABAddressBookAddRecord(_addressBookRef, person.recordRef, &errorRef);
         if (!result){
-            RHLog(@"Error: Failed to add RHPerson to AddressBook: error: %@", (NSError*)errorRef);
+            RHErrorLog(@"Error: Failed to add RHPerson to AddressBook: error: %@", errorRef);
+            if (errorRef) CFRelease(errorRef);
         } else {
             if (![_people containsObject:person])[_people addObject:person];
         }
@@ -639,7 +641,8 @@ -(BOOL)addGroup:(RHGroup *)group{
         CFErrorRef errorRef = NULL;
         result = ABAddressBookAddRecord(_addressBookRef, group.recordRef, &errorRef);
         if (!result){
-            RHLog(@"Error: Failed to add RHGroup to AddressBook: error: %@", (NSError*)errorRef);
+            RHErrorLog(@"Error: Failed to add RHGroup to AddressBook: error: %@", errorRef);
+            if (errorRef) CFRelease(errorRef);
         } else {
             if (![_groups containsObject:group])[_groups addObject:group];
         }
@@ -711,8 +714,11 @@ -(BOOL)removePerson:(RHPerson*)person error:(NSError**)error{
         result = ABAddressBookRemoveRecord(_addressBookRef, person.recordRef, &cfError);
         //if (result)[_people removeObject:person]; //we shouldn't actually remove this object from the cache on removal.. all accesses go via AB record methods so removing now just means the same object is not returned by the cache if the user reverts the removal.
     }];
-
-    if (error) *error = (NSError*)cfError;
+
+    if (!result){
+        if (error) *error = [[(NSError*)cfError retain] autorelease];
+        if (cfError) CFRelease(cfError);
+    }
     return result;
 }
 
@@ -733,8 +739,10 @@ -(BOOL)removeGroup:(RHGroup*)group error:(NSError**)error{
 
     }];
 
-    if (error) *error = (NSError*)cfError;
-
+    if (!result){
+        if (error) *error = [[(NSError*)cfError retain] autorelease];
+        if (cfError) CFRelease(cfError);
+    }    
     return result;
 }
 
@@ -744,7 +752,7 @@ -(BOOL)save{
     NSError *error = nil;
     BOOL result = [self save:&error];
     if (!result) {
-        RHLog(@"RHAddressBook: Error saving: %@", error);
+        RHErrorLog(@"RHAddressBook: Error saving: %@", error);
     }
     return result;
 }
@@ -758,7 +766,11 @@ -(BOOL)save:(NSError**)error{
             result = ABAddressBookSave(_addressBookRef, &cfError);
         }
     }];
-    if (error) *error = (NSError*)cfError;
+
+    if (!result){
+        if (error) *error = [[(NSError*)cfError retain] autorelease];
+        if (cfError) CFRelease(cfError);
+    }
     return result;
 }
 

diff --git a/RHAddressBook/RHAddressBookGeoResult.m b/RHAddressBook/RHAddressBookGeoResult.m
@@ -152,7 +152,7 @@ -(void)geocodeAssociatedAddressDictionary{
 
     //don't do anything if our address is no longer valid
     if (! [self isValid]){ 
-        NSLog(@"ERROR: %@ is no longer valid. Skipping Geocode Op.", self);
+        RHLog(@"%@ is no longer valid. Skipping Geocode Op.", self);
         return;
     }
 

diff --git a/RHAddressBook/RHGroup.m b/RHAddressBook/RHGroup.m
@@ -58,7 +58,7 @@ -(NSString*)name{
 -(void)setName:(NSString *)name{
     NSError *error = nil;
     if (![self setBasicValue:(CFStringRef)name forPropertyID:kABGroupNameProperty error:&error]){
-        RHLog(@"-[RHGroup %@] error:%@", NSStringFromSelector(_cmd), error);
+        RHErrorLog(@"-[RHGroup %@] error:%@", NSStringFromSelector(_cmd), error);
     }
 }
 
@@ -84,15 +84,23 @@ -(BOOL)addMember:(RHPerson*)person{
     [self performRecordAction:^(ABRecordRef recordRef) {
         CFErrorRef errorRef = NULL;
         success = ABGroupAddMember(recordRef, person.recordRef, &errorRef);
-        if (errorRef) NSLog(@"RHGroup: Error adding member. %@", errorRef);
+        if (!success) {
+            RHErrorLog(@"RHGroup: Error adding member. %@", errorRef);
+            if (errorRef) CFRelease(errorRef);
+        }
     } waitUntilDone:YES];
     return success;
 }
 
 -(BOOL)removeMember:(RHPerson*)person{
     __block BOOL success = NO;
     [self performRecordAction:^(ABRecordRef recordRef) {
-        success = ABGroupRemoveMember(recordRef, person.recordRef, nil);
+        CFErrorRef errorRef = NULL;
+        success = ABGroupRemoveMember(recordRef, person.recordRef, &errorRef);
+        if (!success) {
+            RHErrorLog(@"RHGroup: Error removing member. %@", errorRef);
+            if (errorRef) CFRelease(errorRef);
+        }
     } waitUntilDone:YES];
     return success;
 }
@@ -123,7 +131,7 @@ -(NSArray*)members{
             if (person) {
                 [members addObject:person];
             } else {
-                NSLog(@"Failed to find member");
+                RHLog(@"Failed to find member");
             }
         }
 
@@ -150,7 +158,7 @@ -(NSArray*)membersOrderedBySortOrdering:(ABPersonSortOrdering)ordering{
             if (person){
                 [members addObject:person]; 
             } else {
-                NSLog(@"Failed to find member");
+                RHLog(@"Failed to find member");
             }
         }
         CFRelease(memberRefs);