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

Re: svn commit: r1170205 - /subversion/trunk/subversion/libsvn_client/diff.c

From: Hyrum K Wright <hyrum.wright_at_wandisco.com>
Date: Tue, 13 Sep 2011 10:24:23 -0500

On Tue, Sep 13, 2011 at 10:17 AM, Bert Huijben <bert_at_qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: hwright_at_apache.org [mailto:hwright_at_apache.org]
>> Sent: dinsdag 13 september 2011 17:08
>> To: commits_at_subversion.apache.org
>> Subject: svn commit: r1170205 -
>> /subversion/trunk/subversion/libsvn_client/diff.c
>>
>> Author: hwright
>> Date: Tue Sep 13 15:08:29 2011
>> New Revision: 1170205
>>
>> URL: http://svn.apache.org/viewvc?rev=1170205&view=rev
>> Log:
>> Add an iterpool to a loop.
>>
>> * subversion/libsvn_client/diff.c
>>   (display_prop_diffs): Use an iterpool for the internal display loop.
>>
>> Modified:
>>     subversion/trunk/subversion/libsvn_client/diff.c
>>
>> Modified: subversion/trunk/subversion/libsvn_client/diff.c
>> URL:
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/di
>> ff.c?rev=1170205&r1=1170204&r2=1170205&view=diff
>> ==========================================================
>> ====================
>> --- subversion/trunk/subversion/libsvn_client/diff.c (original)
>> +++ subversion/trunk/subversion/libsvn_client/diff.c Tue Sep 13 15:08:29
>> 2011
>
>
>> @@ -680,7 +684,7 @@ display_prop_diffs(const apr_array_heade
>>          }
>>
>>        {
>> -        svn_stream_t *os = svn_stream_from_aprfile2(file, TRUE, pool);
>> +        svn_stream_t *os = svn_stream_from_aprfile2(file, TRUE, iterpool);
>
> Unrelated from this patch, but why do we create a disowned stream here...
>
>>          svn_diff_t *diff;
>>          svn_diff_file_options_t options = { 0 };
>>          const svn_string_t *tmp;
>> @@ -691,14 +695,16 @@ display_prop_diffs(const apr_array_heade
>>             Since the diff is not useful anyway for patching properties an
>>             eol character is appended when needed to remove those pescious
>>             ' \ No newline at end of file' lines. */
>> -        tmp = original_value ? original_value : svn_string_create("", pool);
>> -        orig = maybe_append_eol(tmp, pool);
>> +        tmp = original_value ? original_value : svn_string_create("",
>> +                                                                  iterpool);
>> +        orig = maybe_append_eol(tmp, iterpool);
>>
>>          tmp = propchange->value ? propchange->value :
>> -                                  svn_string_create("", pool);
>> -        val = maybe_append_eol(tmp, pool);
>> +                                  svn_string_create("", iterpool);
>> +        val = maybe_append_eol(tmp, iterpool);
>>
>> -        SVN_ERR(svn_diff_mem_string_diff(&diff, orig, val, &options, pool));
>> +        SVN_ERR(svn_diff_mem_string_diff(&diff, orig, val, &options,
>> +                                         iterpool));
>>
>>          /* UNIX patch will try to apply a diff even if the diff header
>>           * is missing. It tries to be helpful by asking the user for a
>> @@ -708,13 +714,16 @@ display_prop_diffs(const apr_array_heade
>>           * instead of "@@" as the default hunk delimiter for property diffs.
>>           * We also supress the diff header. */
>>          SVN_ERR(svn_diff_mem_string_output_unified2(os, diff, FALSE, "##",
>> -                                           svn_dirent_local_style(path, pool),
>> -                                           svn_dirent_local_style(path, pool),
>> -                                           encoding, orig, val, pool));
>> +                                           svn_dirent_local_style(path,
>> +                                                                  iterpool),
>> +                                           svn_dirent_local_style(path,
>> +                                                                  iterpool),
>> +                                           encoding, orig, val, iterpool));
>>          SVN_ERR(svn_stream_close(os));
>
> ... and then explicitly close it here.
>
> This makes this close a no-op

Yeah, the stream handling in the diff code is kinda...interesting.

What prompted this particular change was my poking around in the diff
code to see if we could use output streams instead of output files.
In my current patch, the above stream handling is gone.

-Hyrum

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Received on 2011-09-13 17:24:56 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.