David Madore <[EMAIL PROTECTED]> wrote: > Yes, I am willing to do that.
Great. I'll send you the copyright forms. >> My requesting a `clean' patch means we're pretty picky, >> and that the more of the following you can do, the better. > > Can I ask you to take a look at the attached patch? It is not yet > satisfactory (at least, it needs to be forwardported to the CVS > version, as I worked against 5.2.1 because I wanted to install this > ASAP on my Debian boxen), but maybe we can start from this to discuss > what needs to be done. It implements sha224, sha256, sha384 and > sha512. > > [Note: need to "chmod +x tests/sha???sum/basic-1" after applying.] > > Here are a few things which need to be said about it, and which may be > concerns: > > - sha224 and sha256 (in lib/sha256.c) present no particular > difficulties. However, as concerns sha384 and sha512 we have the > following problem: the algorithm uses 64-bit words. The attached > patch (in lib/sha512.c) tries to find a 64-bit unsigned integer type > basically by copying the code from md5/sha1 and adding a test for > unsigned long long (and it barfs if it can't find one): however, this > is probably not portable enough for your goals. It should be possible Actually, it may not be a big deal at all. If there is a reasonable porting target that lacks a 64-bit integral type, people will report it and we can address the problem then. Regarding this comment, +/* The following contortions are an attempt to use the C preprocessor + to determine an unsigned integral type that is 64 bits wide. An + alternative approach is to use autoconf's AC_CHECK_SIZEOF macro, but + doing that would require that the configure script compile and *run* + the resulting executable. Locally running cross-compiled executables + is usually not possible. */ FWIW, we *can* use compile-only tests to check for type sizes. We should be able to write a 64-bit analog of m4/uint32_t.m4. There may already be such a macro. But for now, you're welcome to go ahead with what you have. Besides, it'll take at least a couple of weeks to do the copyright paper exchange. BTW, it's ok to use #error, now. > to write an implementation of sha384 and sha512 using only 32-bit > words, but that would be extremely tedious, so I might be willing to > do it, but not immediately. That doesn't seem worthwhile. > In the meantime, I guess one needs to > have some configure test determine whether 64-bit words are available > and, if not, warn the user and skip compiling sha384 and sha512. Only > I don't know how to do that, so I'll need some help here. > > - Speaking of word sizes, there's a potential alignment problem in the > common code in src/md5sum.c, which does not seem to check whether the > result buffer is 32-bit aligned (let alone 64-bit). This should be > trivial to fix, if that's an issue: but someone should decide whether > the fix goes in the library routines (remove alignment constraint) or > in the md5sum common code. As far as alignment, I haven't heard of any problems, but maybe that's just luck, since that buffer is the first automatic variable declared in main. You're welcome to fix it in src/md5sum.c. > - sha1sum does not seem to be documented in texinfo (at least not in > 5.2.1), so, since I was lazily copying everything I could from sha1, I > didn't come up with a texinfo doc for sha224 etc. I'm willing to try > to document sha1sum along with the rest (but I've never written > texinfo, I hope that's not a problem). Thanks! We still do need texinfo documentation for sha1sum. > - With respect to the following remark of yours: > >> By the way, your e.g., src/sha256sum.c file should look like this: >> >> #include "checksum.h" >> int algorithm = ALG_SHA256; ... > So I'd like to advocate doing things differently, as in my attached > patch: do away with src/md5.c, src/sha1sum.c and src/checksum.h, and > simply compile src/md5sum.c six times, with a different compile-time > flag each time, to provide the six utilities. This works fine in > automake, it means recompiling the same source six times but the > resulting utilities are smaller and do not contain any needless code. > How does that sound? That sounds fine, of course. Obviously I didn't really want 6 copies of such code in each binary :) The point was to continue to keep everything well factored, as you've done. > - Lastly, as concerns tests, I used the FIPS-supplied test vectors, > except that I did not include the "one million a's" test, because I > don't know how to do this properly with Fetish (we don't really want > to write a file with one millon a's in it just to test the utility: it > would be much better to pipe the data, but I don't know the framework > for that). These days, I wouldn't worry about creating a 1MB file. You could use the Fetish (now, in cvs, called Coreutils.pm) framework with 'a'.1000000 as the input. Or use the tests/sample template and do something like this to get the checksum: printf %1000000s ' '|sed 's/ /a/' |sha256sum > out -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]