You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2009/05/23 12:24:22 UTC

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

Neels Janosch Hofmeyr wrote on Fri, 22 May 2009 at 19:54 -0700:
> Author: neels
> Date: Fri May 22 19:54:22 2009
> New Revision: 37795
> 
> Log:
> Fix issue #3404: Normalize line endings in svn:* props during svnsync.
> Always automatically convert all svn:* properties to LF line ending style,
> and notify about all normalizations collectively after all else is done.
> 

Thanks for taking this fix in!

Reviewing for backport, it looks good, modulo four places that miss an
SVN_ERR() wrap:

> * subversion/svnsync/main.c
>   (normalize_string, normalize_revprops, log_properties_normalized):
>     Add functions.

> @@ -684,7 +787,10 @@ do_initialize(svn_ra_session_t *to_sessi
> +  /* Notify about normalized props, if any. */
> +  log_properties_normalized(normalized_rev_props_count, 0, pool);
>  

One,

> @@ -1016,6 +1123,15 @@ change_file_prop(void *file_baton,
> +  /* Normalize svn:* properties as necessary. */
> +  if (svn_prop_needs_translation(name))
> +    {
> +      svn_boolean_t was_normalized;
> +      normalize_string(&value, &was_normalized, pool);

two,

> @@ -1104,6 +1220,16 @@ change_dir_prop(void *dir_baton,
> +  /* Normalize svn:* properties as necessary. */
> +  if (svn_prop_needs_translation(name))
> +    {
> +      svn_boolean_t was_normalized;
> +      normalize_string((const svn_string_t**)&real_value,
> +                       &was_normalized, pool);

three,

> @@ -1706,11 +1863,16 @@ do_copy_revprops(svn_ra_session_t *to_se
> +  /* Notify about normalized props, if any. */
> +  log_properties_normalized(normalized_rev_props_count, 0, pool);
> +
>    return SVN_NO_ERROR;
>  }

four.


I'll go and vote in STATUS now.


Thanks (and sorry that I didn't have time to fix it myself),

Daniel

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2353148

Re: svnsync fails with segmentation fault trying to normalize a null string

Posted by Neels Janosch Hofmeyr <ne...@elego.de>.
Hi Alexander,

thanks a lot for this report.
The fix is trivial, committed in r37939.

Good speed with svnkit!
~Neels

Alexander Sinyushkin wrote:
> Hello community,
> 
> While syncing a repository where svn:eol-style was first set on a file
> and then removed in a subsequent revision I got a segmentation fault. As 
> a result, svnsync can not synchronize the revision where the deletion of 
> an svn:* property took place.
> 
> When change_file_prop() is called for the file in question
> the property value is passed as null, I suppose.
> This value is passed then to normalize_string() which does not check the 
> str parameter for null in its if clause:
> 
> static svn_error_t*
> normalize_string(const svn_string_t **str,
>                   svn_boolean_t *was_normalized,
>                   apr_pool_t *pool)
> {
>    *was_normalized = FALSE;
> 
>    /* Detect inconsistent line ending style simply by looking
>       for carriage return (\r) characters. */
>    if (strchr((*str)->data, '\r') != NULL)
>      {
>        /* Found some. Normalize. */
>        const char* cstring = NULL;
>        SVN_ERR(svn_subst_translate_cstring2((*str)->data, &cstring,
>                                             "\n", TRUE,
>                                             NULL, FALSE,
>                                             pool));
>        *str = svn_string_create(cstring, pool);
>        *was_normalized = TRUE;
>      }
> 
>    return SVN_NO_ERROR;
> }
> 
> 
> ----
> Alexander Sinyushkin,
> TMate Software,
> http://svnkit.com/ - Java [Sub]Versioning Library!
> 
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2359689

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2359733

Re: svnsync fails with segmentation fault trying to normalize a null string

Posted by Mark Phippard <ma...@gmail.com>.
I believe this change to normalize the strings has gone into 1.6.3.
So perhaps we should get a fix for this in place before we roll the
release?

Mark



On Fri, Jun 5, 2009 at 8:34 AM, Alexander
Sinyushkin<Al...@svnkit.com> wrote:
> Hello community,
>
> While syncing a repository where svn:eol-style was first set on a file
> and then removed in a subsequent revision I got a segmentation fault. As
> a result, svnsync can not synchronize the revision where the deletion of
> an svn:* property took place.
>
> When change_file_prop() is called for the file in question
> the property value is passed as null, I suppose.
> This value is passed then to normalize_string() which does not check the
> str parameter for null in its if clause:
>
> static svn_error_t*
> normalize_string(const svn_string_t **str,
>                  svn_boolean_t *was_normalized,
>                  apr_pool_t *pool)
> {
>   *was_normalized = FALSE;
>
>   /* Detect inconsistent line ending style simply by looking
>      for carriage return (\r) characters. */
>   if (strchr((*str)->data, '\r') != NULL)
>     {
>       /* Found some. Normalize. */
>       const char* cstring = NULL;
>       SVN_ERR(svn_subst_translate_cstring2((*str)->data, &cstring,
>                                            "\n", TRUE,
>                                            NULL, FALSE,
>                                            pool));
>       *str = svn_string_create(cstring, pool);
>       *was_normalized = TRUE;
>     }
>
>   return SVN_NO_ERROR;
> }
>
>
> ----
> Alexander Sinyushkin,
> TMate Software,
> http://svnkit.com/ - Java [Sub]Versioning Library!
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2359689
>



-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2359699


svnsync fails with segmentation fault trying to normalize a null string

Posted by Alexander Sinyushkin <Al...@svnkit.com>.
Hello community,

While syncing a repository where svn:eol-style was first set on a file
and then removed in a subsequent revision I got a segmentation fault. As 
a result, svnsync can not synchronize the revision where the deletion of 
an svn:* property took place.

When change_file_prop() is called for the file in question
the property value is passed as null, I suppose.
This value is passed then to normalize_string() which does not check the 
str parameter for null in its if clause:

static svn_error_t*
normalize_string(const svn_string_t **str,
                  svn_boolean_t *was_normalized,
                  apr_pool_t *pool)
{
   *was_normalized = FALSE;

   /* Detect inconsistent line ending style simply by looking
      for carriage return (\r) characters. */
   if (strchr((*str)->data, '\r') != NULL)
     {
       /* Found some. Normalize. */
       const char* cstring = NULL;
       SVN_ERR(svn_subst_translate_cstring2((*str)->data, &cstring,
                                            "\n", TRUE,
                                            NULL, FALSE,
                                            pool));
       *str = svn_string_create(cstring, pool);
       *was_normalized = TRUE;
     }

   return SVN_NO_ERROR;
}


----
Alexander Sinyushkin,
TMate Software,
http://svnkit.com/ - Java [Sub]Versioning Library!

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2359689

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

Posted by Neels Janosch Hofmeyr <ne...@elego.de>.
Daniel Shahaf wrote:
> Neels Janosch Hofmeyr wrote on Fri, 22 May 2009 at 19:54 -0700:
>> Author: neels
>> Date: Fri May 22 19:54:22 2009
>> New Revision: 37795
>>
>> Log:
>> Fix issue #3404: Normalize line endings in svn:* props during svnsync.
>> Always automatically convert all svn:* properties to LF line ending style,
>> and notify about all normalizations collectively after all else is done.
>>
> 
> Thanks for taking this fix in!
> 
> Reviewing for backport, it looks good, modulo four places that miss an
> SVN_ERR() wrap:

 -- ouch  %P
Thanks for catching it. r37799. Added to backport nomination of r37795.
~Neels

> 
>> * subversion/svnsync/main.c
>>   (normalize_string, normalize_revprops, log_properties_normalized):
>>     Add functions.
> 
>> @@ -684,7 +787,10 @@ do_initialize(svn_ra_session_t *to_sessi
>> +  /* Notify about normalized props, if any. */
>> +  log_properties_normalized(normalized_rev_props_count, 0, pool);
>>  
> 
> One,
> 
>> @@ -1016,6 +1123,15 @@ change_file_prop(void *file_baton,
>> +  /* Normalize svn:* properties as necessary. */
>> +  if (svn_prop_needs_translation(name))
>> +    {
>> +      svn_boolean_t was_normalized;
>> +      normalize_string(&value, &was_normalized, pool);
> 
> two,
> 
>> @@ -1104,6 +1220,16 @@ change_dir_prop(void *dir_baton,
>> +  /* Normalize svn:* properties as necessary. */
>> +  if (svn_prop_needs_translation(name))
>> +    {
>> +      svn_boolean_t was_normalized;
>> +      normalize_string((const svn_string_t**)&real_value,
>> +                       &was_normalized, pool);
> 
> three,
> 
>> @@ -1706,11 +1863,16 @@ do_copy_revprops(svn_ra_session_t *to_se
>> +  /* Notify about normalized props, if any. */
>> +  log_properties_normalized(normalized_rev_props_count, 0, pool);
>> +
>>    return SVN_NO_ERROR;
>>  }
> 
> four.
> 
> 
> I'll go and vote in STATUS now.
> 
> 
> Thanks (and sorry that I didn't have time to fix it myself),
> 
> Daniel
> 
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2353148

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2353227