4
\$\begingroup\$

I'd like to truncate 160-bit output from SHA-1 to receive a (weaker) 64-bit digest.

It has been a while since I did the type of low-level C pointer arithmetic in the but-last line. Could you please review it for me? It should return the last 64 bits of the the byte array in a uint64_t. (I guess conceptually I could just as well take the first 64 bits, which would make the code simpler but here we are.)

#import <CommonCrypto/CommonDigest.h>

- (uint64_t)digestFromString: (NSString *)s {
    NSData *input = [s dataUsingEncoding: NSUTF8StringEncoding];
    unsigned char output[CC_SHA1_DIGEST_LENGTH];
    CC_SHA1(input.bytes, (CC_LONG)input.length, output);
    uint64_t digest;
    NSAssert(CC_SHA1_DIGEST_LENGTH >= sizeof(uint64_t), nil);
    digest = *(uint64_t *)(output + CC_SHA1_DIGEST_LENGTH - sizeof(uint64_t));
    return digest;
}

How could this code be improved upon?

UPDATE Here is a second (alternative) version with an attempt at avoiding any alignment issues. (Its much more straigtforward to truncate to the first 64 bits in this case.) Actual digests will differ from the 1st version.

- (uint64_t)digestFromString: (NSString *)s {
    NSData *input = [s dataUsingEncoding: NSUTF8StringEncoding];
    union { // uint64_t-aligned
        uint64_t      uint64s[CC_SHA1_DIGEST_LENGTH / sizeof(uint64_t) + sizeof(uint64_t)];
        unsigned char bytes[CC_SHA1_DIGEST_LENGTH];
    } output;
    CC_SHA1(input.bytes, (CC_LONG)input.length, (unsigned char *)output.bytes);
    return output.uint64s[0];
}
\$\endgroup\$
  • \$\begingroup\$ I've gone ahead and removed the C tag from this question, since this code would not be accepted by a C compiler. That tag is for use with code that is strictly just C. \$\endgroup\$ – syb0rg Feb 3 '15 at 19:57
  • \$\begingroup\$ @syb0rg Fair enough. (I'm new to this forum and learning about its conventions.) \$\endgroup\$ – Drux Feb 3 '15 at 19:59
  • 1
    \$\begingroup\$ stackoverflow.com/questions/13881487/… - you have (at least theoretical) alignment issues with that cast. \$\endgroup\$ – Mat Feb 3 '15 at 20:00
  • \$\begingroup\$ @Mat Good catch. I've posted a 2nd version. \$\endgroup\$ – Drux Feb 3 '15 at 20:20
  • 2
    \$\begingroup\$ @nhgrif: As I understand it, the second method is a alternative/revised version of the first method to avoid the potential alignment issues mentioned in the comments. \$\endgroup\$ – Martin R Feb 3 '15 at 22:09
6
\$\begingroup\$

Both your methods should work. As far as I know, there is no alignment problem on the current OS X and iOS architectures in your first method (see below).

The code from the second method can be simplified a tiny bit:

- (uint64_t)digestFromString: (NSString *)s {
    NSData *input = [s dataUsingEncoding: NSUTF8StringEncoding];
    union { // uint64_t-aligned
        uint64_t      digest;
        unsigned char bytes[CC_SHA1_DIGEST_LENGTH];
    } output;
    CC_SHA1(input.bytes, (CC_LONG)input.length, output.bytes);
    return output.digest;
}

The first element in the union need not be an array, and casting output.bytes to unsigned char * is not necessary (the array automatically becomes a pointer to the first element when passed as a function argument).

But the easiest method to extract a 64-bit integer from the digest would be to use the inline function

OS_INLINE uint64_t _OSReadInt64(
    const volatile void               * base,
    uintptr_t                     byteOffset
)

from OSByteOrder.h that comes with the OS X and iOS SDKs:

uint64_t digest = _OSReadInt64(output, CC_SHA1_DIGEST_LENGTH - sizeof(uint64_t)); // last 8 bytes
uint64_t digest = _OSReadInt64(output, 0); // first 8 bytes

This function is implemented as

return *(volatile uint64_t *)((uintptr_t)base + byteOffset);

which indicates that there is no alignment problem when casting an arbitrary pointer to uint64_t *. But that may be different on future architectures, with using the above function you are on the safe side.

Alternatively, you can use memcpy:

uint64_t digest;
memcpy(&digest, output, sizeof digest);

Instead of

NSAssert(CC_SHA1_DIGEST_LENGTH >= sizeof(uint64_t), nil);

you could use a "compile-time/static assertion":

_Static_assert(CC_SHA1_DIGEST_LENGTH >= sizeof(uint64_t), "");

_Static_assert is a GCC extension which is also understood by the Clang compiler, so you can use it in your Xcode projects.

The advantage is that it causes a compiler error if the condition is not satisfied, instead of failing later at runtime.

\$\endgroup\$
  • \$\begingroup\$ A very solid answer, thx. Just one more point about _OSReadInt64: the current implementation (if I read it correctly) does not seem to prevent against all alignment issues. Are you saying that _OSReadInt64 is safe against such issues because Apple may provide a different implementation if needed in the future? \$\endgroup\$ – Drux Feb 4 '15 at 7:35
  • \$\begingroup\$ @Drux: yes, that's how I understand it. \$\endgroup\$ – Martin R Feb 4 '15 at 7:58
  • \$\begingroup\$ @Drux: Btw, is your code for OS X or for iOS devices? I found a reference that unaligned access on the ARM processor works, but is very slow, so thinking about it again I would now recommend the memcpy() method in the first place. – I will update the answer later, when I have more time. \$\endgroup\$ – Martin R Feb 4 '15 at 8:19
  • \$\begingroup\$ It's for iOS (but in principle should be portable). BTW, I now like your adaptation of my 2nd version best (first 8 bytes, aligned, w/o memcpy()). \$\endgroup\$ – Drux Feb 4 '15 at 8:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.