Quincey, I did some changes following your recommendations, this is my current version:
+(NSString *)generateHashFromPath:(NSString *)path { const NSUInteger CHUNK_SIZE = 65536; NSData *fileData = [NSData dataWithContentsOfFile:path options:NSDataReadingMapped | NSDataReadingUncached error:NULL]; char buffer[CHUNK_SIZE]; CC_MD5_CTX md5; CC_MD5_Init(&md5); [fileData getBytes:buffer range:NSMakeRange(0, CHUNK_SIZE)]; CC_MD5_Update(&md5, buffer, CHUNK_SIZE); [fileData getBytes:buffer range:NSMakeRange(MAX(0, [fileData length] - CHUNK_SIZE), CHUNK_SIZE)]; CC_MD5_Update(&md5, buffer, CHUNK_SIZE); unsigned char digest[CC_MD5_DIGEST_LENGTH]; CC_MD5_Final(digest, &md5); NSMutableString *ret = [NSMutableString stringWithCapacity:CC_MD5_DIGEST_LENGTH * 2]; for (int i = 0; i < CC_MD5_DIGEST_LENGTH; i++) { [ret appendFormat:@"%02x", digest[i]]; } return [ret lowercaseString]; } Any other tip for improving it? (I know, I still need to handle the errors, hehe, but anything else?) --- Wilker LĂșcio http://about.me/wilkerlucio/bio Kajabi Consultant +55 81 82556600 On Wed, Jun 29, 2011 at 10:31 AM, Wilker <wilkerlu...@gmail.com> wrote: > Thanks Quincey, these things for sure will be a lot helpful, Im still > pretty new to Objective-C, but I will look in all of those things that you > said, thanks :) > > --- > Wilker LĂșcio > http://about.me/wilkerlucio/bio > Kajabi Consultant > +55 81 82556600 > > > > On Wed, Jun 29, 2011 at 5:13 AM, Quincey Morris < > quinceymor...@earthlink.net> wrote: > >> On Jun 29, 2011, at 00:27, Quincey Morris wrote: >> >> >> [fileData appendData:[file readDataOfLength:CHUNK_SIZE]]; >> >> [file seekToFileOffset:MAX(0, fileSize - CHUNK_SIZE)]; >> >> [fileData appendData:[file readDataOfLength:CHUNK_SIZE]]; >> >> >> >> [file closeFile]; >> > >> > This is not the worst way to read the entire contents of a file into a >> NSData object, but it's pretty bad. :) >> > >> > It would be far easier and more efficient to use [NSData >> dataWithContentsOfFile: path options: NSDataReadingMapped | >> NSDataReadingUncached error: NULL]; >> >> Er, two things. >> >> I didn't notice until after I posted that you weren't reading the whole >> file. So, sorry, "pretty bad" was too strong -- I'm upgrading my response to >> "not the best". :) >> >> I think the way I suggested is still the best way, because (typically) >> using the NSDataReadingMapped means the file contents are mapped into >> virtual memory, so pages (4KB, or whatever the VM page size is) are not >> actually read until you try to access them. >> >> If you use CC_MD5_Init/Update (first part)/Update (last part)/Final >> instead of the convenience method that does it all, only the parts of the >> file you need will actually be read. >> >> NSDataReadingMapped is in general the most efficient way to read a file >> when you only need to read any part once, I think. >> >> The problem with NSFileHandle is that it pretty much sucks as a class. It >> doesn't return any errors, so if something goes wrong it's either going to >> throw an exception or just ignore it. IIRC it has no API contract regarding >> error reporting. >> >> Second thing... >> >> I forgot to point out that your code has a glaring bug. With your >> declarations, 'fileSize - CHUNK_SIZE' in 'MAX(0, fileSize - CHUNK_SIZE)' is >> an unsigned expression. It can't go negative, and so that statement isn't >> doing what it looks like it's doing. >> >> One more thing ... >> >> Since you have no release/retain calls, I'm assuming you're using garbage >> collection. If that's the case, then this line is very, very dangerous: >> >> > return [SMEncodingUtilities md5HexDigestFromChar:[fileData bytes] >> withLength:(unsigned int)[fileData length]]; >> >> You're passing an *interior* pointer to fileData's private storage, but >> the (compiler-optimized) lifetime of 'fileData' ends *before* the md5 method >> is entered, and so the object is subject to being collected *before* you've >> used the pointer. This will crash. >> >> In your code, the window of danger is pretty small. But it will crash >> eventually. >> >> The solution is to put a reference to the object after the call: >> >> > NSString* result = [SMEncodingUtilities >> md5HexDigestFromChar:[fileData bytes] withLength:(unsigned int)[fileData >> length]]; >> > [fileData self]; >> > return result; >> >> It's a PITA, but you gotta be careful with NSData. >> >> > _______________________________________________ Cocoa-dev mailing list (Cocoa-dev@lists.apple.com) Please do not post admin requests or moderator comments to the list. Contact the moderators at cocoa-dev-admins(at)lists.apple.com Help/Unsubscribe/Update your Subscription: http://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com This email sent to arch...@mail-archive.com