From dev-return-8037-daniel=haxx.se@subversion.apache.org Wed Dec 1 04:27:31 2010 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on giant.haxx.se X-Spam-Level: X-Spam-Status: No, score=-4.5 required=3.0 tests=BAYES_00,DS_FRIEND, T_RP_MATCHES_RCVD autolearn=ham version=3.3.1 Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by giant.haxx.se (8.14.3/8.14.3/Debian-9.1) with SMTP id oB13RUxU018912 for ; Wed, 1 Dec 2010 04:27:30 +0100 Received: (qmail 19927 invoked by uid 500); 1 Dec 2010 03:27:24 -0000 Mailing-List: contact dev-help@subversion.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list dev@subversion.apache.org Received: (qmail 19919 invoked by uid 99); 1 Dec 2010 03:27:23 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 01 Dec 2010 03:27:23 +0000 X-ASF-Spam-Status: No, hits=-0.7 required=10.0 tests=RCVD_IN_DNSWL_LOW,SPF_PASS Received-SPF: pass (nike.apache.org: local policy) Received: from [66.111.4.25] (HELO out1.smtp.messagingengine.com) (66.111.4.25) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 01 Dec 2010 03:27:14 +0000 Received: from compute3.internal (compute3.nyi.mail.srv.osa [10.202.2.43]) by gateway1.messagingengine.com (Postfix) with ESMTP id 9344B239; Tue, 30 Nov 2010 22:26:53 -0500 (EST) Received: from frontend1.messagingengine.com ([10.202.2.160]) by compute3.internal (MEProxy); Tue, 30 Nov 2010 22:26:53 -0500 X-Sasl-enc: gkQ4KL8yHEFprJh2IdbG8Y+4othR3UG6wk6xv8dRZ7sBAhRVOlwRQ1xVNytbgg 1291174012 Received: from daniel3.local (bzq-79-180-20-182.red.bezeqint.net [79.180.20.182]) by mail.messagingengine.com (Postfix) with ESMTPSA id 6D38B407F37; Tue, 30 Nov 2010 22:26:52 -0500 (EST) Date: Wed, 1 Dec 2010 05:25:28 +0200 From: Daniel Shahaf To: dev@subversion.apache.org Cc: Stefan Fuhrmann Subject: Re: svn commit: r1040831 - in /subversion/trunk/subversion: include/svn_checksum.h libsvn_subr/checksum.c Message-ID: <20101201032528.GI8686@daniel3.local> References: <20101130235641.5B68123889ED@eris.apache.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20101130235641.5B68123889ED@eris.apache.org> User-Agent: Mutt/1.5.18 (2008-05-17) X-Virus-Checked: Checked by ClamAV on apache.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.3.5 (giant.haxx.se [80.67.6.50]); Wed, 01 Dec 2010 04:27:31 +0100 (CET) X-Friend: Friend 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. > * 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: > >