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

[Greg Stein <gs...@gmail.com>]

On Mon, Aug 18, 2008 at 9:47 AM,  <hw...@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@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org