subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Fuhrmann <stefanfuhrm...@alice-dsl.de>
Subject Re: svn commit: r1040831 - in /subversion/trunk/subversion: include/svn_checksum.h libsvn_subr/checksum.c
Date Thu, 02 Dec 2010 00:39:34 GMT
On 01.12.2010 04:25, Daniel Shahaf wrote:
> stefan2@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.
Sure:

$svnadmin-1.5.4 create /mnt/svnroot/test
$<add pre-revprop-change hook>
$svnsync-1.5.4 init file:///mnt/svnroot/test http://svn.apache.org/repos/asf
$svnsync-1.5.4 sync file:///mnt/svnroot/test
$<cancel after a few revs; rev 1 will already trigger the error>
$svnadmin-trunk verify /mnt/svnroot/test
>> * 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.)
>
Done.

-- Stefan^2.
>>    * @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:
>>
>>


Mime
View raw message