Paul Burba wrote:
> On Fri, Jun 5, 2009 at 10:40 AM, Neels Janosch Hofmeyr<neels_at_elego.de> wrote:
>> Author: neels
>> Date: Fri Jun 5 07:40:07 2009
>> New Revision: 37939
>>
>> Log:
>> Fix segfault from r37795 and add regression test.
>> The segfault occurs when svnsync encounters deletion of an
>> "svn:*" property.
>>
>> * subversion/svnsync/main.c
>> (normalize_string): Check for NULL pointer.
>>
>> * subversion/tests/cmdline/svnsync_tests_data/delete-svn-props.dump:
>> New dump file for svnsync cmdline tests that involves the removal of
>> an "svn:eol-style" file-property (s.b.).
>>
>> * subversion/tests/cmdline/svnsync_tests.py
>> (delete_svn_props): New test for this segfault, using the dump file.
>>
>> The dump file above was created with the lines:
>> [[[
>> set -x
>> dir=repos
>> url="file://`pwd`/$dir"
>> propname=svn:eol-style
>> propval=LF
>> rm -rf ./$dir
>> rm -rf ./WC
>> svnadmin create $dir
>> svn co $url WC
>> echo file-content > WC/file
>> svn add WC/file
>> svn ps $propname $propval WC/file
>> svn ci -m "adding file with prop '$propname'" WC
>> svn pd $propname WC/file
>> svn ci -m "removing prop '$propname' from file" WC
>> svnadmin dump $dir > delete-svn-props.dump
>> ]]]
>>
>> Added:
>> trunk/subversion/tests/cmdline/svnsync_tests_data/delete-svn-props.dump
>> Modified:
>> trunk/subversion/svnsync/main.c
>> trunk/subversion/tests/cmdline/svnsync_tests.py
>>
>> Modified: trunk/subversion/svnsync/main.c
>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svnsync/main.c?pathrev=37939&r1=37938&r2=37939
>> ==============================================================================
>> --- trunk/subversion/svnsync/main.c Fri Jun 5 07:19:08 2009 (r37938)
>> +++ trunk/subversion/svnsync/main.c Fri Jun 5 07:40:07 2009 (r37939)
>> @@ -529,6 +529,9 @@ normalize_string(const svn_string_t **st
>> {
>> *was_normalized = FALSE;
>>
>> + if (*str == NULL || (*str)->data == NULL)
>
> Hi Neels,
>
> Do we really need the second NULL check? (*str)->data should never be
> NULL per svn_string_t's 1st invariant. If we are passing around an
> svn_string_t with a NULL data member I don't think we should ignore
> it.
>
> Paul
...oh, so it *was* the right way 'round in hyrum's mail.
Ok, as you put it, the second check should be stricter. Making it an assert.
Thanks for pointing it out. r37946, updated STATUS.
~Neels
>
> * @file svn_string.h
> * @brief Counted-length strings for Subversion, plus some C string goodies.
> *
> * There are two string datatypes: @c svn_string_t and @c svn_stringbuf_t.
> * The former is a simple pointer/length pair useful for passing around
> * strings (or arbitrary bytes) with a counted length. @c svn_stringbuf_t is
> * buffered to enable efficient appending of strings without an allocation
> * and copy for each append operation.
> *
> * @c svn_string_t contains a <tt>const char *</tt> for its data, so it is
> * most appropriate for constant data and for functions which expect constant,
> * counted data. Functions should generally use <tt>const @c svn_string_t
> * *</tt> as their parameter to indicate they are expecting a constant,
> * counted string.
> *
> * @c svn_stringbuf_t uses a plain <tt>char *</tt> for its data, so it is
> * most appropriate for modifiable data.
> *
> * <h3>Invariants</h3>
> *
> * 1. Null termination:
> *
> * Both structures maintain a significant invariant:
> *
> * <tt>s->data[s->len] == '\\0'</tt>
> *
> * The functions defined within this header file will maintain
> * the invariant (which does imply that memory is
> * allocated/defined as @c len+1 bytes). If code outside of the
> * @c svn_string.h functions manually builds these structures,
> * then they must enforce this invariant.
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2359813
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2359884
Received on 2009-06-06 02:46:58 CEST