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
