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

Re: svn commit: r37939 - in trunk/subversion: svnsync tests/cmdline tests/cmdline/svnsync_tests_data

From: Paul Burba <ptburba_at_gmail.com>
Date: Fri, 5 Jun 2009 15:41:51 -0400

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

 * @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
Received on 2009-06-05 21:42:13 CEST

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