[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1040831 - in /subversion/trunk/subversion: include/svn_checksum.h libsvn_subr/checksum.c

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Wed, 1 Dec 2010 05:25:28 +0200

stefan2_at_apache.org wrote on Tue, Nov 30, 2010 at 23:56:41 -0000:
> Author: stefan2
> Date: Tue Nov 30 23:56:40 2010
> New Revision: 1040831
>
> URL: http://svn.apache.org/viewvc?rev=1040831&view=rev
> Log:
> Fix 'svnadmin verify' for format 5 repositories. During the checking process,
> they yield NULL for SHA1 checksums. The most robust way to fix that is to
> allow NULL for checksum in svn_checksum_to_cstring. This seems justified
> as NULL is already a valid return value instead of e.g. an empty string. So,
> callers may receive NULL as result and could therefore assume that NULL is
> a valid input, too
>

Can you explain how to trigger the bug you are fixing? Just running
'svnadmin verify' on my r1040058-created Greek repository doesn't.

> * subversion/include/svn_checksum.h
> (svn_checksum_to_cstring): extend doc string
> * subversion/libsvn_subr/checksum.c
> (svn_checksum_to_cstring): return NULL for NULL checksums as well
>
> Modified:
> subversion/trunk/subversion/include/svn_checksum.h
> subversion/trunk/subversion/libsvn_subr/checksum.c
>
> Modified: subversion/trunk/subversion/include/svn_checksum.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_checksum.h?rev=1040831&r1=1040830&r2=1040831&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_checksum.h (original)
> +++ subversion/trunk/subversion/include/svn_checksum.h Tue Nov 30 23:56:40 2010
> @@ -121,7 +121,7 @@ svn_checksum_to_cstring_display(const sv
>
> /** Return the hex representation of @a checksum, allocating the
> * string in @a pool. If @a checksum->digest is all zeros (that is,
> - * 0, not '0'), then return NULL.
> + * 0, not '0') or NULL, then return NULL.
> *

First, this change doesn't match the code change: the docstring change
says CHECKSUM->DIGEST may be NULL, but the code change allows CHECKSUM
to be NULL.

Second, let's have the docstring say that NULL is only allowed by 1.7+.
(Passing NULL will segfault in 1.6.)

(doxygen has a @note directive.)

> * @since New in 1.6.
> */
>
> Modified: subversion/trunk/subversion/libsvn_subr/checksum.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/checksum.c?rev=1040831&r1=1040830&r2=1040831&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/checksum.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/checksum.c Tue Nov 30 23:56:40 2010
> @@ -135,6 +135,9 @@ const char *
> svn_checksum_to_cstring(const svn_checksum_t *checksum,
> apr_pool_t *pool)
> {
> + if (checksum == NULL)
> + return NULL;
> +
> switch (checksum->kind)
> {
> case svn_checksum_md5:
>
>
Received on 2010-12-01 04:27:31 CET

This is an archived mail posted to the Subversion Dev mailing list.