[pacman-dev] [PATCH] Clean up MD5 code.

[Dan McGee]

On 7/25/07, Andrew Fyfe <andrew at neptune-one.net> wrote:
>         * Move alpm md5 functions to lib/libalpm/util.c
>         * Remove unneeded includes for md5.h
>         * Replace md5 implementation with one from http://www.xyssl.org
>
> Signed-off-by: Andrew Fyfe <andrew at neptune-one.net>
> ---
>  lib/libalpm/Makefile.am |    1 -
>  lib/libalpm/add.c       |    7 +-
>  lib/libalpm/md5.c       |  661 +++++++++++++++++++++++++++++------------------
>  lib/libalpm/md5.h       |  144 ++++++++---
>  lib/libalpm/md5driver.c |   93 -------
>  lib/libalpm/remove.c    |    1 -
>  lib/libalpm/sync.c      |    1 -
>  lib/libalpm/util.c      |   31 +++
>  8 files changed, 547 insertions(+), 392 deletions(-)

I've now pulled both the SHA1 removal patch and this one into my
working branch. However, this one needed a few fixes which should be
reflected in the diff I hacked up below.

Two major things to point out in the diff:
1. Even on one line if/for/looping statements, use {}. This is pacman
coding style and helps keep us consistent, and it cuts out stupid
bugs.
2. Watch your mallocs, and use calloc when possible. You didn't
allocate space for the null byte, so you were overrunning your buffers
when you filled them and the free() failed when using mtrace(). I
switched to calloc usage, and now use sprintf because this is a case
where we can do that- it is faster and we aren't worried about running
out of room. We then need to take care of the null byte ourselves,
however.

I'll give you a break on some of this because you are venturing into C
code where few have gone before, and you probably weren't aware of the
rules. I think this is the most recent version of them:
http://www.archlinux.org/~aaron/pacman-coding.html

Finally, I cleaned up the imported md5.c/md5.h from XySSL a bit. I
removed the HMAC and SELF_CHECK stuff we won't use, as well as threw
the LGPL header at the top of md5.h and put instructions for upgrading
the md5 routines in md5.c.

-Dan


diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
index 0f47e90..5f43117 100644
--- a/lib/libalpm/util.c
+++ b/lib/libalpm/util.c
@@ -550,26 +551,32 @@ char SYMEXPORT *alpm_get_md5sum(char *filename)
 {
 	unsigned char output[16];
 	char *md5sum;
+	int ret, i;

 	ALPM_LOG_FUNC;

 	ASSERT(filename != NULL, return(NULL));

-	md5sum = (char*)malloc(32);
-	int ret = md5_file(filename, output);
+	/* allocate 32 chars plus 1 for null */
+	md5sum = calloc(33, sizeof(char));
+	ret = md5_file(filename, output);

 	if (ret > 0) {
-		if (ret == 1)
+		if (ret == 1) {
 			_alpm_log(PM_LOG_ERROR, _("md5: %s can't be opened\n"), filename);
-		else if (ret == 2)
+		} else if (ret == 2) {
 			_alpm_log(PM_LOG_ERROR, _("md5: %s can't be read\n"), filename);
+		}

 		return(NULL);
 	}

 	/* Convert the result to something readable */
-	for (unsigned int i = 0; i < 16; i++)
-		snprintf(md5sum + i * 2, 33, "%02x", output[i]);
+	for (i = 0; i < 16; i++) {
+		/* sprintf is acceptable here because we know our output */
+		sprintf(md5sum +(i * 2), "%02x", output[i]);
+	}
+	md5sum[32] = '\0';

 	_alpm_log(PM_LOG_DEBUG, "md5(%s) = %s", filename, md5sum);
 	return(md5sum);