Re: svn commit: r32524 - in trunk: . subversion/include subversion/libsvn_subr subversion/tests/libsvn_subr

On Mon, Aug 18, 2008 at 9:47 AM, <hwright_at_tigris.org> wrote:
>...
> +++ trunk/subversion/libsvn_subr/checksum.c Mon Aug 18 09:47:20 2008 (r32524)
>...
> +svn_checksum_create(svn_checksum_kind_t kind,
> apr_pool_t *pool)
> {
> - *checksum = apr_palloc(pool, sizeof(*checksum));
> + svn_checksum_t *checksum = apr_palloc(pool, sizeof(*checksum));

I think you could reduce this to one allocation:

svn_checksum_t *checksum = apr_palloc(pool, sizeof(*checksum) +
APR_MD5_DIGESTSIZE);
checksum->digest = (unsigned char *)checksum + APR_MD5_DIGESTSIZE;

Put that into each block of the switch.

Also, shouldn't the digest be zero'd out? (if so, then use apr_pcalloc)

>...
> +svn_checksum_clear(svn_checksum_t *checksum)
> +{
> + switch (checksum->kind)
> + {
> + case svn_checksum_md5:
> + memset(checksum->digest, 0, APR_MD5_DIGESTSIZE);
> + break;
> +
> + case svn_checksum_sha1:
> + memset(checksum->digest, 0, APR_SHA1_DIGESTSIZE);

Might be handy to have a macro that returns the digest size given the
kind. It could be used in numerous places, I think.

#define whatever(kind) (kind == svn_checksum_md5 ? APR_MD5_DIGESTSIZE
: (kind == svn_checksum_sha1 ? APR_SHA1_DIGESTSIZE : 0))

>...
> + baton->read_more = read_all;

Why the change in name?

>...

Cheers,
-g

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-18 22:55:26 CEST