Re: generic crypto - remarks

[Simon Josefsson]

Paul Eggert <address@hidden> writes:

> Jim Meyering <address@hidden> writes:
>
>>   verify (offsetof (struct S, member_m) % 4 == 0);
>>
>> Hmm... that assumes 8-bit bytes.
>
> And it also assumes no holes in integer representations.
> This is more portable:
>
>   verify (offsetof (struct S, member_m) % alignof (uint32_t) == 0);
>
> where alignof is defined as with md5.c etc.

The "verify" module is GPL.  If you re-license it, md4 would be able
to use it.

> But I agree with Bruno; it'd be better to change the type of the
> following member from char to uint32_t to avoid all this hassle.

The following patch works.  What do you think?  Should I install this?
And similar to the MD5 and SHA-1 module too?

Frankly, I think it should be possible to implement MD4 (and MD5, and
SHA-1) without these casts and alignment checks.  Is the rest of the
file really portable?  The code look rather messy to me.

Thanks.

Index: md4.h
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/md4.h,v
retrieving revision 1.1
diff -u -p -r1.1 md4.h
--- md4.h       18 Oct 2005 22:59:17 -0000      1.1
+++ md4.h       22 Oct 2005 15:29:13 -0000
@@ -22,7 +22,7 @@
 # include <stdio.h>
 # include <stdint.h>
 
-#define MD4_DIGEST_SIZE 16
+# define MD4_DIGEST_SIZE 16
 
 /* Structure to save state of computation between the single steps.  */
 struct md4_ctx
@@ -34,7 +34,7 @@ struct md4_ctx
 
   uint32_t total[2];
   uint32_t buflen;
-  char buffer[128] __attribute__ ((__aligned__ (__alignof__ (uint32_t))));
+  uint32_t buffer[128];
 };
 
 
Index: md4.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/md4.c,v
retrieving revision 1.3
diff -u -p -r1.3 md4.c
--- md4.c       21 Oct 2005 12:46:48 -0000      1.3
+++ md4.c       22 Oct 2005 15:29:13 -0000
@@ -100,12 +100,12 @@ md4_finish_ctx (struct md4_ctx *ctx, voi
     ++ctx->total[1];
 
   pad = bytes >= 56 ? 64 + 56 - bytes : 56 - bytes;
-  memcpy (&ctx->buffer[bytes], fillbuf, pad);
+  memcpy (&((char*)ctx->buffer)[bytes], fillbuf, pad);
 
   /* Put the 64-bit file length in *bits* at the end of the buffer.  */
-  *(uint32_t *) &ctx->buffer[bytes + pad] = SWAP (ctx->total[0] << 3);
-  *(uint32_t *) &ctx->buffer[bytes + pad + 4] = SWAP ((ctx->total[1] << 3) |
-                                                     (ctx->total[0] >> 29));
+  ctx->buffer[(bytes + pad) / 4] = SWAP (ctx->total[0] << 3);
+  ctx->buffer[(bytes + pad) / 4 + 1] = SWAP ((ctx->total[1] << 3) |
+                                            (ctx->total[0] >> 29));
 
   /* Process last bytes.  */
   md4_process_block (ctx->buffer, bytes + pad + 8, ctx);
@@ -208,7 +208,7 @@ md4_process_bytes (const void *buffer, s
       size_t left_over = ctx->buflen;
       size_t add = 128 - left_over > len ? len : 128 - left_over;
 
-      memcpy (&ctx->buffer[left_over], buffer, add);
+      memcpy (&((char*)ctx->buffer)[left_over], buffer, add);
       ctx->buflen += add;
 
       if (ctx->buflen > 64)
@@ -217,7 +217,7 @@ md4_process_bytes (const void *buffer, s
 
          ctx->buflen &= 63;
          /* The regions in the following copy operation cannot overlap.  */
-         memcpy (ctx->buffer, &ctx->buffer[(left_over + add) & ~63],
+         memcpy (ctx->buffer, &((char*)ctx->buffer)[(left_over + add) & ~63],
                  ctx->buflen);
        }
 
@@ -240,7 +240,7 @@ md4_process_bytes (const void *buffer, s
       if (UNALIGNED_P (buffer))
        while (len > 64)
          {
-           md4_process_block (memcpy (ctx->buffer, buffer, 64), 64, ctx);
+           md4_process_block (memcpy (ctx->buffer, buffer, 16), 64, ctx);
            buffer = (const char *) buffer + 64;
            len -= 64;
          }
@@ -258,13 +258,13 @@ md4_process_bytes (const void *buffer, s
     {
       size_t left_over = ctx->buflen;
 
-      memcpy (&ctx->buffer[left_over], buffer, len);
+      memcpy (&((char*)ctx->buffer)[left_over], buffer, len);
       left_over += len;
       if (left_over >= 64)
        {
          md4_process_block (ctx->buffer, 64, ctx);
          left_over -= 64;
-         memcpy (ctx->buffer, &ctx->buffer[64], left_over);
+         memcpy (ctx->buffer, &ctx->buffer[16], left_over);
        }
       ctx->buflen = left_over;
     }