Re: problem with applying md5 to data
Re: problem with applying md5 to data
- Subject: Re: problem with applying md5 to data
- From: Wilker <email@hidden>
- Date: Wed, 29 Jun 2011 11:08:15 -0300
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:@"x", 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 <email@hidden> 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 <
> email@hidden> 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 (email@hidden)
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:
This email sent to email@hidden