From 1108135232e292e3f496e40b455318e2ab1ef4fd Mon Sep 17 00:00:00 2001 From: rfm Date: Fri, 3 Jan 2025 16:05:09 +0000 Subject: [PATCH] Issue #305 ... produce NSError with 516 code on copy failure because destination already exists. --- Source/NSError.m | 19 ++++ Source/NSFileManager.m | 169 ++++++++++++++++++----------- Tests/base/NSFileManager/general.m | 18 ++- 3 files changed, 131 insertions(+), 75 deletions(-) diff --git a/Source/NSError.m b/Source/NSError.m index dd77b146f..0bf860408 100644 --- a/Source/NSError.m +++ b/Source/NSError.m @@ -24,6 +24,7 @@ #import "common.h" #define EXPOSE_NSError_IVARS 1 #import "Foundation/NSDictionary.h" +#import "Foundation/NSException.h" #import "Foundation/NSError.h" #import "Foundation/NSCoder.h" #import "Foundation/NSArray.h" @@ -32,10 +33,28 @@ @implementation NSError +/* For NSFileManager we have a private method which produces an error + * with mutable userInfo so that information can be added before the + * file manager returns the error to higher level code. + */ ++ (NSError*) _error: (NSInteger)aCode + description: (NSString*)description +{ + NSError *e = [self allocWithZone: NSDefaultMallocZone()]; + NSMutableDictionary *m; + + e = [e initWithDomain: NSCocoaErrorDomain code: aCode userInfo: nil]; + m = [NSMutableDictionary allocWithZone: NSDefaultMallocZone()]; + e->_userInfo = [m initWithCapacity: 3]; + [m setObject: description forKey: NSLocalizedDescriptionKey]; + return AUTORELEASE(e); +} + + (id) errorWithDomain: (NSErrorDomain)aDomain code: (NSInteger)aCode userInfo: (NSDictionary*)aDictionary { + NSError *e = [self allocWithZone: NSDefaultMallocZone()]; e = [e initWithDomain: aDomain code: aCode userInfo: aDictionary]; diff --git a/Source/NSFileManager.m b/Source/NSFileManager.m index 55552ebde..873235435 100644 --- a/Source/NSFileManager.m +++ b/Source/NSFileManager.m @@ -43,6 +43,7 @@ #import "common.h" #define EXPOSE_NSFileManager_IVARS 1 #define EXPOSE_NSDirectoryEnumerator_IVARS 1 +#import "Foundation/FoundationErrors.h" #import "Foundation/NSArray.h" #import "Foundation/NSAutoreleasePool.h" #import "Foundation/NSData.h" @@ -177,6 +178,11 @@ #define GSBINIO 0 #endif +@interface NSError (NSFileManager) ++ (NSError*) _error: (NSInteger)aCode + description: (NSString*)description; +@end + @interface NSDirectoryEnumerator (Local) - (id) initWithDirectoryPath: (NSString*)path recurseIntoSubdirectories: (BOOL)recurse @@ -322,8 +328,10 @@ exitedThread(void *lastErrorPtr) toPath: (NSString*) toPath; /* Thread-local last error variable. */ -- (NSString*) _lastError; -- (void) _setLastError: (NSString*)error; +- (NSError*) _lastError; +- (NSString*) _lastErrorText; +- (void) _setLastError: (NSString*)msg code: (int)code; +- (void) _setLastError: (NSString*)msg; /* A convenience method to return an NSError object. * If the _lastError message is set, this creates an NSError using @@ -388,7 +396,7 @@ static gs_mutex_t classLock = GS_MUTEX_INIT_STATIC; - (void) dealloc { - [self _setLastError: nil]; + [self _setLastError: nil code: 0]; [super dealloc]; } @@ -831,7 +839,7 @@ static gs_mutex_t classLock = GS_MUTEX_INIT_STATIC; NSDirectoryEnumerator *direnum; NSString *path; - [self _setLastError: nil]; + [self _setLastError: nil code: 0]; if (![[url scheme] isEqualToString: @"file"]) { @@ -952,7 +960,7 @@ static gs_mutex_t classLock = GS_MUTEX_INIT_STATIC; BOOL shouldRecurse; BOOL shouldSkipHidden; - [self _setLastError: nil]; + [self _setLastError: nil code: 0]; if (![[url scheme] isEqualToString: @"file"]) { @@ -996,7 +1004,7 @@ static gs_mutex_t classLock = GS_MUTEX_INIT_STATIC; { NSArray *result; - [self _setLastError: nil]; + [self _setLastError: nil code: 0]; result = [self directoryContentsAtPath: path]; @@ -1026,7 +1034,7 @@ static gs_mutex_t classLock = GS_MUTEX_INIT_STATIC; { BOOL result = NO; - [self _setLastError: nil]; + [self _setLastError: nil code: 0]; if (YES == flag) { @@ -1344,13 +1352,15 @@ static gs_mutex_t classLock = GS_MUTEX_INIT_STATIC; if ([self fileExistsAtPath: destination] == YES) { - [self _setLastError: @"Could not copy - destination already exists"]; + [self _setLastError: @"Could not copy - destination already exists" + code: NSFileWriteFileExistsError]; return NO; } attrs = [self fileAttributesAtPath: source traverseLink: NO]; if (attrs == nil) { - [self _setLastError: @"Cound not copy - destination is not readable"]; + [self _setLastError: @"Could not copy - source is not available" + code: NSFileReadNoSuchFileError]; return NO; } fileType = [attrs fileType]; @@ -1386,7 +1396,7 @@ static gs_mutex_t classLock = GS_MUTEX_INIT_STATIC; if ([self createDirectoryAtPath: destination attributes: attrs] == NO) { if (NO == [self _proceedAccordingToHandler: handler - forError: [self _lastError] + forError: [self _lastErrorText] inPath: destination fromPath: source toPath: destination]) @@ -1458,7 +1468,7 @@ static gs_mutex_t classLock = GS_MUTEX_INIT_STATIC; { BOOL result; - [self _setLastError: nil]; + [self _setLastError: nil code: 0]; result = [self copyPath: src toPath: dst handler: nil]; @@ -1578,7 +1588,7 @@ static gs_mutex_t classLock = GS_MUTEX_INIT_STATIC; { BOOL result; - [self _setLastError: nil]; + [self _setLastError: nil code: 0]; result = [self movePath: src toPath: dst handler: nil]; @@ -1657,7 +1667,7 @@ static gs_mutex_t classLock = GS_MUTEX_INIT_STATIC; if ([self createDirectoryAtPath: destination attributes: attrs] == NO) { return [self _proceedAccordingToHandler: handler - forError: [self _lastError] + forError: [self _lastErrorText] inPath: destination fromPath: source toPath: destination]; @@ -1821,7 +1831,7 @@ static gs_mutex_t classLock = GS_MUTEX_INIT_STATIC; { BOOL result; - [self _setLastError: nil]; + [self _setLastError: nil code: 0]; result = [self removeFileAtPath: path handler: nil]; @@ -1848,7 +1858,7 @@ static gs_mutex_t classLock = GS_MUTEX_INIT_STATIC; { BOOL result; - [self _setLastError: nil]; + [self _setLastError: nil code: 0]; result = [self createSymbolicLinkAtPath: path pathContent: destPath]; @@ -2309,7 +2319,7 @@ static gs_mutex_t classLock = GS_MUTEX_INIT_STATIC; { NSDictionary *d; - [self _setLastError: nil]; + [self _setLastError: nil code: 0]; d = [GSAttrDictionaryClass attributesAt: path traverseLink: NO]; @@ -3429,7 +3439,7 @@ static inline void gsedRelease(GSEnumeratedDirectory X) if (dirOK == NO) { if (![self _proceedAccordingToHandler: handler - forError: [self _lastError] + forError: [self _lastErrorText] inPath: destinationFile fromPath: sourceFile toPath: destinationFile]) @@ -3535,7 +3545,7 @@ static inline void gsedRelease(GSEnumeratedDirectory X) attributes: attributes] == NO) { if ([self _proceedAccordingToHandler: handler - forError: [self _lastError] + forError: [self _lastErrorText] inPath: destinationFile fromPath: sourceFile toPath: destinationFile] == NO) @@ -3646,80 +3656,109 @@ static inline void gsedRelease(GSEnumeratedDirectory X) return NO; } -- (NSString*) _lastError +- (NSError*) _lastError { #if __has_feature(objc_arc) - return (__bridge NSString*)GS_THREAD_KEY_GET(thread_last_error_key); + return (__bridge NSError*)GS_THREAD_KEY_GET(thread_last_error_key); #else - return (NSString*)GS_THREAD_KEY_GET(thread_last_error_key); + return (NSError*)GS_THREAD_KEY_GET(thread_last_error_key); #endif } -- (void) _setLastError: (NSString*)error +- (NSString*) _lastErrorText { + return [[[self _lastError] userInfo] objectForKey: NSLocalizedDescriptionKey]; +} + +- (void) _setLastError: (NSString*)msg code: (int)aCode +{ + NSError *oldError; + NSError *error; + + if (msg) + { + error = [NSError _error: aCode description: msg]; + } + else + { + error = nil; + } #if __has_feature(objc_arc) - NSString *oldError = (__bridge_transfer NSString*)GS_THREAD_KEY_GET(thread_last_error_key); + oldError = (__bridge_transfer NSError*)GS_THREAD_KEY_GET(thread_last_error_key); GS_THREAD_KEY_SET(thread_last_error_key, (__bridge_retained void*)error); #pragma unused(oldError) #else - NSString *oldError = (NSString*)GS_THREAD_KEY_GET(thread_last_error_key); + oldError = (NSError*)GS_THREAD_KEY_GET(thread_last_error_key); GS_THREAD_KEY_SET(thread_last_error_key, RETAIN(error)); RELEASE(oldError); #endif } +- (void) _setLastError: (NSString*)msg +{ + [self _setLastError: msg code: 0]; +} + - (NSError*) _errorFrom: (NSString *)fromPath to: (NSString *)toPath { - NSError *error; - NSDictionary *errorInfo; - NSString *message; - NSString *domain; - NSInteger code; - NSString *lastError = [self _lastError]; + NSError *error = [self _lastError]; - if (lastError) + if (error) { - message = lastError; - domain = NSCocoaErrorDomain; - code = 0; + NSMutableDictionary *m; + + /* The last error was created using a private method which produces + * an error instance with a mutable dictionary as its userInfo so we + * can efficiently use that error to return additional information. + */ + error = AUTORELEASE(RETAIN(error)); + [self _setLastError: nil]; + m = (NSMutableDictionary*)[error userInfo]; + if (fromPath && toPath) + { + [m setObject: fromPath forKey: @"FromPath"]; + [m setObject: toPath forKey: @"ToPath"]; + } + else if (fromPath) + { + [m setObject: fromPath forKey: NSFilePathErrorKey]; + } } else { + NSDictionary *errorInfo; + NSString *message; + NSString *domain; + NSInteger code; + error = [NSError _last]; message = [error localizedDescription]; domain = [error domain]; code = [error code]; - } - - if (fromPath && toPath) - { - errorInfo = [NSDictionary dictionaryWithObjectsAndKeys: - fromPath, @"FromPath", - toPath, @"ToPath", - message, NSLocalizedDescriptionKey, - nil]; - } - else if (fromPath) - { - errorInfo = [NSDictionary dictionaryWithObjectsAndKeys: - fromPath, NSFilePathErrorKey, - message, NSLocalizedDescriptionKey, - nil]; - } - else - { - errorInfo = [NSDictionary dictionaryWithObjectsAndKeys: - message, NSLocalizedDescriptionKey, - nil]; - } - - error = [NSError errorWithDomain: domain - code: code - userInfo: errorInfo]; - - if (lastError) - { - [self _setLastError: nil]; + if (fromPath && toPath) + { + errorInfo = [NSDictionary dictionaryWithObjectsAndKeys: + fromPath, @"FromPath", + toPath, @"ToPath", + message, NSLocalizedDescriptionKey, + nil]; + } + else if (fromPath) + { + errorInfo = [NSDictionary dictionaryWithObjectsAndKeys: + fromPath, NSFilePathErrorKey, + message, NSLocalizedDescriptionKey, + nil]; + } + else + { + errorInfo = [NSDictionary dictionaryWithObjectsAndKeys: + message, NSLocalizedDescriptionKey, + nil]; + } + error = [NSError errorWithDomain: domain + code: code + userInfo: errorInfo]; } return error; diff --git a/Tests/base/NSFileManager/general.m b/Tests/base/NSFileManager/general.m index c4aa36e66..343184945 100644 --- a/Tests/base/NSFileManager/general.m +++ b/Tests/base/NSFileManager/general.m @@ -1,13 +1,6 @@ #import "Testing.h" #import "ObjectTesting.h" -#import -#import -#import -#import -#import -#import -#import -#import +#import #ifdef EQ #undef EQ @@ -413,9 +406,14 @@ int main() error: &err]; PASS([mgr fileExistsAtPath: @"sub2/sub1" isDirectory: &isDir] && isDir == YES, "NSFileManager copy item at URL"); - [mgr copyItemAtPath: @"sub2" toPath: @"sub1/sub2" error: &err]; + PASS([mgr copyItemAtPath: @"sub2" toPath: @"sub1/sub2" error: &err] == YES + && nil == err, "NSFileManager copy item at Path returns expected values") PASS([mgr fileExistsAtPath: @"sub1/sub2/sub1" isDirectory: &isDir] - && isDir == YES, "NSFileManager copy item at Path"); + && isDir == YES, "NSFileManager copy item at Path actually works"); + PASS([mgr copyItemAtPath: @"sub2" toPath: @"sub1/sub2" error: &err] == NO + && nil != err, "NSFileManager copy item at Path fails when dest exists") + PASS([err code] == NSFileWriteFileExistsError, "expected error code") + [mgr moveItemAtURL: [NSURL fileURLWithPath: @"sub2/sub1"] toURL: [NSURL fileURLWithPath: @"sub1/moved"] error: &err];