[libdvdnav-devel] [PATCH] src/md5.c: Fix potential strict-aliasing breakage

[Andrew Clayton]

With GCC 4.8.3 on x86_64 Fedora 20 we get the following warnings from
src/md5.c

    src/md5.c: In function 'md5_finish_ctx':
    src/md5.c:102:3: warning: dereferencing type-punned pointer will break
      strict-aliasing rules [-Wstrict-aliasing]
     *(md5_uint32 *) &ctx->buffer[bytes + pad] = SWAP (ctx->total[0] << 3);
     ^
    src/md5.c:103:3: warning: dereferencing type-punned pointer will break
      strict-aliasing rules [-Wstrict-aliasing]
     *(md5_uint32 *) &ctx->buffer[bytes + pad + 4] = SWAP ((ctx->total[1]
      << 3) |
     ^
ctx->total is a 2 element array of type md5_uint32 and ctx->buffer is a
character array. So the code is dereferencing and casting ctx->buffer to
a different pointer type.

This code is used by the DVDDiscID() function

    /**
     * Get a unique 128 bit disc ID.
     * This is the MD5 sum of VIDEO_TS.IFO and the VTS_0?_0.IFO files
     * in title order (those that exist).

I got rid of this casting and dereference by using memcpy() to copy the
total stuff into buffer.

This doesn't seem to be used by libdvdread or indeed vlc or lsdvd, so to
test this I hacked up a small test program

    /*
     * gcc -O2 -Wall -o dvddiscid dvddiscid.c -ldvdread
     */

    #include <stdio.h>

    #include <dvdread/dvd_reader.h>

    int main(int argc, char *argv[])
    {
	    int i;
	    dvd_reader_t *dvd;
	    unsigned char id[16];

	    dvd = DVDOpen("/dev/sr0");
	    DVDDiscID(dvd, id);
	    DVDClose(dvd);

	    for (i = 0; i < 16; i++)
		    printf("%x", id[i]);
	    printf("\n");

	    return 0;
    }

Testing a DVD gives me the same result both before and after this
change.

Signed-off-by: Andrew Clayton <andrew at digital-domain.net>
---
 src/md5.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/md5.c b/src/md5.c
index 9c5f50d..607a067 100644
--- a/src/md5.c
+++ b/src/md5.c
@@ -88,6 +88,7 @@ md5_finish_ctx (ctx, resbuf)
 {
   /* Take yet unprocessed bytes into account.  */
   md5_uint32 bytes = ctx->buflen;
+  md5_uint32 bits;
   size_t pad;
 
   /* Now count remaining bytes.  */
@@ -99,9 +100,10 @@ md5_finish_ctx (ctx, resbuf)
   memcpy (&ctx->buffer[bytes], fillbuf, pad);
 
   /* Put the 64-bit file length in *bits* at the end of the buffer.  */
-  *(md5_uint32 *) &ctx->buffer[bytes + pad] = SWAP (ctx->total[0] << 3);
-  *(md5_uint32 *) &ctx->buffer[bytes + pad + 4] = SWAP ((ctx->total[1] << 3) |
-                                                        (ctx->total[0] >> 29));
+  bits = SWAP (ctx->total[0] << 3);
+  memcpy (&ctx->buffer[bytes + pad], &bits, sizeof (bits));
+  bits = SWAP ((ctx->total[1] << 3) | (ctx->total[0] >> 29));
+  memcpy (&ctx->buffer[bytes + pad + 4], &bits, sizeof (bits));
 
   /* Process last bytes.  */
   md5_process_block (ctx->buffer, bytes + pad + 8, ctx);
-- 
1.9.3