Skip to content

Commit d7774c0

Browse files
Sean McBrideSean McBride
authored andcommitted
performed a code review, specifically:
- changed some #include to #import - fixed some NULL -> nil Cocoa coding conventions - added new compiler warnings and fixed some warnings they generated - check for nil from NSTemporaryDirectory - added missing files to unit test and test app targets - added xcconfig files for unit test target - added @Private to some ivars - changes some variables from signed to unsigned as appropriate - changed from base 2 to base 10 measurements of file size, consistent with both the actual meaning of metric prefixes and Apple's new policy as of 10.6 - reduced some unneeded copy-paste of code - fixed failure to check for null from malloc and unneeded check against null before calling free - OSErr was incorrectly used instead of OSStatus - added some consts & statics to global strings - fixed some issues discovered by static analysis - fixed some 64bit issues, mostly related to casting and the use of slightly incorrect types/sizes - some dealloc methods were using accessors, changed to access ivars directly, as per Apple guidelines - removed old NS_DURING, NS_HANDLER, NS_ENDHANDLER macros - fixed a bug where immutable data was being mutated - removed all instance of "== YES" as they are dangerous - removed some redundant nil checks - fixed some leaks - conditionally replaced deprecated method usage - cleanup CF/NSMakeCollectable usage - fixed bug in GC where memory could be collected too early due to lack of strong references when using UTF8String - prevent passing null to CFRelease
1 parent bd80144 commit d7774c0

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+307
-281
lines changed

Configurations/ConfigCommon.xcconfig

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ GCC_DEBUGGING_SYMBOLS = full
1414
GCC_PRECOMPILE_PREFIX_HEADER = YES
1515
GCC_PREFIX_HEADER = $(SDKROOT)/System/Library/Frameworks/Cocoa.framework/Headers/Cocoa.h
1616
GCC_FAST_OBJC_DISPATCH = YES
17+
GCC_ENABLE_PASCAL_STRINGS = NO
1718
ARCHS = ppc i386 x86_64
1819

1920
// Enable warnings
@@ -36,4 +37,15 @@ GCC_WARN_UNKNOWN_PRAGMAS = YES
3637
GCC_WARN_UNUSED_VARIABLE = YES
3738
GCC_WARN_ABOUT_DEPRECATED_FUNCTIONS = YES
3839
GCC_WARN_ABOUT_INVALID_OFFSETOF_MACRO = YES
39-
WARNING_CFLAGS = -Wall
40+
GCC_WARN_ABOUT_MISSING_PROTOTYPES = YES
41+
GCC_WARN_HIDDEN_VIRTUAL_FUNCTIONS = YES
42+
GCC_WARN_ABOUT_INVALID_OFFSETOF_MACRO = YES
43+
GCC_WARN_UNUSED_FUNCTION = YES
44+
GCC_WARN_UNUSED_LABEL = YES
45+
GCC_WARN_UNUSED_VALUE = YES
46+
GCC_WARN_UNUSED_PARAMETER = YES
47+
GCC_WARN_ABOUT_MISSING_FIELD_INITIALIZERS = YES
48+
WARNING_CFLAGS = -Wall -Wundef -Wendif-labels -Wpointer-arith -Wcast-align -Wwrite-strings -Wmissing-noreturn -Wmissing-format-attribute -Wpacked -Wredundant-decls -Winline -Wdisabled-optimization -Wformat=2 -Wlarger-than-32768 -Winvalid-pch
49+
50+
// TODO:
51+
// GCC_WARN_UNDECLARED_SELECTOR = YES only 7 warnings

Configurations/ConfigCommonDebug.xcconfig

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,8 @@
33
GCC_OPTIMIZATION_LEVEL = 0
44
DEBUG_INFORMATION_FORMAT = dwarf
55
GCC_GENERATE_DEBUGGING_SYMBOLS = YES
6-
SPARKLE_EXTRA_DEBUG = -DDEBUG -fstack-protector -D_FORTIFY_SOURCE=2
7-
OTHER_CFLAGS = $(SPARKLE_EXTRA_DEBUG)
6+
SPARKLE_EXTRA_DEBUG_10_5_ONLY = -fstack-protector -D_FORTIFY_SOURCE=2
7+
SPARKLE_EXTRA_DEBUG = -DDEBUG
8+
OTHER_CFLAGS = $(SPARKLE_EXTRA_DEBUG)
9+
10+
// Add $(SPARKLE_EXTRA_DEBUG_10_5_ONLY) to SPARKLE_EXTRA_DEBUG if your deployment is 10.5 or greater.
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// Unit Test only
2+
3+
INFOPLIST_FILE = Tests/Sparkle Unit Tests-Info.plist
4+
OTHER_LDFLAGS = -framework Cocoa -framework SenTestingKit
5+
PRODUCT_NAME = Sparkle Unit Tests
6+
WRAPPER_EXTENSION = octest
7+
FRAMEWORK_SEARCH_PATHS = $(DEVELOPER_LIBRARY_DIR)/Frameworks
8+
GCC_PREFIX_HEADER = $(SYSTEM_LIBRARY_DIR)/Frameworks/Cocoa.framework/Headers/Cocoa.h
9+
GCC_PRECOMPILE_PREFIX_HEADER = YES
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
#include "ConfigCommon.xcconfig"
2+
#include "ConfigCommonDebug.xcconfig"
3+
#include "ConfigUnitTest.xcconfig"
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
#include "ConfigCommon.xcconfig"
2+
#include "ConfigCommonRelease.xcconfig"
3+
#include "ConfigUnitTest.xcconfig"
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
#include "ConfigUnitTestRelease.xcconfig"
2+
3+
GCC_ENABLE_OBJC_GC = required

NTSynchronousTask.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
@interface NTSynchronousTask : NSObject
1313
{
14+
@private
1415
NSTask *mv_task;
1516
NSPipe *mv_outputPipe;
1617
NSPipe *mv_inputPipe;

NTSynchronousTask.m

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,10 @@ - (void)dealloc
5656
{
5757
[[NSNotificationCenter defaultCenter] removeObserver:self];
5858

59-
[self setTask:nil];
60-
[self setOutputPipe:nil];
61-
[self setInputPipe:nil];
62-
[self setOutput:nil];
59+
[mv_task release];
60+
[mv_outputPipe release];
61+
[mv_inputPipe release];
62+
[mv_output release];
6363

6464
[super dealloc];
6565
}
@@ -70,7 +70,7 @@ + (NSData*)task:(NSString*)toolPath directory:(NSString*)currentDirectory withAr
7070
NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
7171
NSData* result=nil;
7272

73-
NS_DURING
73+
@try
7474
{
7575
NTSynchronousTask* task = [[NTSynchronousTask alloc] init];
7676

@@ -81,8 +81,7 @@ + (NSData*)task:(NSString*)toolPath directory:(NSString*)currentDirectory withAr
8181

8282
[task release];
8383
}
84-
NS_HANDLER;
85-
NS_ENDHANDLER;
84+
@catch (NSException *localException) { }
8685

8786
[pool drain];
8887

@@ -92,10 +91,6 @@ + (NSData*)task:(NSString*)toolPath directory:(NSString*)currentDirectory withAr
9291
return result;
9392
}
9493

95-
@end
96-
97-
@implementation NTSynchronousTask (Private)
98-
9994
- (void)run:(NSString*)toolPath directory:(NSString*)currentDirectory withArgs:(NSArray*)args input:(NSData*)input;
10095
{
10196
BOOL success = NO;
@@ -118,12 +113,12 @@ - (void)run:(NSString*)toolPath directory:(NSString*)currentDirectory withArgs:(
118113

119114
[[[self outputPipe] fileHandleForReading] readToEndOfFileInBackgroundAndNotifyForModes:[NSArray arrayWithObjects:NSDefaultRunLoopMode, NSModalPanelRunLoopMode, NSEventTrackingRunLoopMode, nil]];
120115

121-
NS_DURING
116+
@try
117+
{
122118
[[self task] launch];
123119
success = YES;
124-
NS_HANDLER
125-
;
126-
NS_ENDHANDLER
120+
}
121+
@catch (NSException *localException) { }
127122

128123
if (success)
129124
{

SUAppcast.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@
1010
#define SUAPPCAST_H
1111

1212
@class SUAppcastItem;
13-
@interface SUAppcast : NSObject {
13+
@interface SUAppcast : NSObject
14+
{
15+
@private
1416
NSArray *items;
1517
NSString *userAgentString;
1618
id delegate;

SUAppcast.m

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,12 @@ - (void)fetchAppcastFromURL:(NSURL *)url
4040

4141
- (void)download:(NSURLDownload *)download decideDestinationWithSuggestedFilename:(NSString *)filename
4242
{
43-
NSString *destinationFilename = [NSTemporaryDirectory() stringByAppendingPathComponent:filename];
44-
[download setDestination:destinationFilename allowOverwrite:NO];
43+
NSString* destinationFilename = NSTemporaryDirectory();
44+
if (destinationFilename)
45+
{
46+
destinationFilename = [destinationFilename stringByAppendingPathComponent:filename];
47+
[download setDestination:destinationFilename allowOverwrite:NO];
48+
}
4549
}
4650

4751
- (void)download:(NSURLDownload *)download didCreateDestination:(NSString *)path
@@ -52,7 +56,8 @@ - (void)download:(NSURLDownload *)download didCreateDestination:(NSString *)path
5256

5357
- (void)downloadDidFinish:(NSURLDownload *)download
5458
{
55-
CFRelease(download);
59+
if (download)
60+
CFRelease(download);
5661

5762
NSError *error = nil;
5863
NSXMLDocument *document = [[NSXMLDocument alloc] initWithContentsOfURL:[NSURL fileURLWithPath:downloadFilename] options:0 error:&error];
@@ -62,7 +67,7 @@ - (void)downloadDidFinish:(NSURLDownload *)download
6267
#if MAC_OS_X_VERSION_MIN_REQUIRED <= MAC_OS_X_VERSION_10_4
6368
[[NSFileManager defaultManager] removeFileAtPath:downloadFilename handler:nil];
6469
#else
65-
[[NSFileManager defaultManager] removeItemAtPath:downloadFilename error:NULL];
70+
[[NSFileManager defaultManager] removeItemAtPath:downloadFilename error:nil];
6671
#endif
6772
[downloadFilename release];
6873
downloadFilename = nil;
@@ -179,13 +184,14 @@ - (void)downloadDidFinish:(NSURLDownload *)download
179184

180185
- (void)download:(NSURLDownload *)download didFailWithError:(NSError *)error
181186
{
182-
CFRelease(download);
187+
if (download)
188+
CFRelease(download);
183189
if (downloadFilename)
184190
{
185191
#if MAC_OS_X_VERSION_MIN_REQUIRED <= MAC_OS_X_VERSION_10_4
186192
[[NSFileManager defaultManager] removeFileAtPath:downloadFilename handler:nil];
187193
#else
188-
[[NSFileManager defaultManager] removeItemAtPath:downloadFilename error:NULL];
194+
[[NSFileManager defaultManager] removeItemAtPath:downloadFilename error:nil];
189195
#endif
190196
}
191197
[downloadFilename release];
@@ -219,7 +225,7 @@ - (NSXMLNode *)bestNodeInNodes:(NSArray *)nodes
219225
NSXMLElement *node;
220226
NSMutableArray *languages = [NSMutableArray array];
221227
NSString *lang;
222-
NSInteger i;
228+
NSUInteger i;
223229
while ((node = [nodeEnum nextObject]))
224230
{
225231
lang = [[node attributeForName:@"xml:lang"] stringValue];

0 commit comments

Comments
 (0)