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

Re: svn commit: r38518 - in trunk/subversion: include libsvn_wc

From: Greg Stein <gstein_at_gmail.com>
Date: Sat, 1 Aug 2009 15:04:14 +0200

On Sat, Aug 1, 2009 at 15:01, Greg Stein<gstein_at_gmail.com> wrote:
> On Sat, Aug 1, 2009 at 07:47, Hyrum K. Wright<hyrum_at_hyrumwright.org> wrote:
>>...
>> +++ trunk/subversion/libsvn_wc/revision_status.c        Fri Jul 31 22:47:16 2009        (r38518)
>>...
>> @@ -95,9 +101,12 @@ svn_wc_revision_status(svn_wc_revision_s
>>   const svn_delta_editor_t *editor;
>>   void *edit_baton;
>>   svn_revnum_t edit_revision;
>> -  svn_wc_revision_status_t *result = apr_palloc(pool, sizeof(**result_p));
>> +  svn_wc_revision_status_t *result = apr_palloc(result_pool,
>> +                                                sizeof(**result_p));
>>   *result_p = result;
>
> I realize you didn't into this, but the sizeof() above is very bad
> form. It is the size of a *different* variable. Sure, it happens to be
> the same size, but "happens" and "is definitely" are two different
> things.
>
>>...
>> @@ -108,14 +117,14 @@ svn_wc_revision_status(svn_wc_revision_s
>>   /* initialize walking baton */
>>   sb.result = result;
>>   sb.committed = committed;
>> -  sb.wc_path = wc_path;
>> +  sb.local_abspath = local_abspath;
>>   sb.wc_url = NULL;
>> -  sb.pool = pool;
>> +  sb.pool = scratch_pool;
>>
>>   SVN_ERR(svn_wc_adm_open_anchor(&anchor_access, &target_access, &target,
>> -                                 wc_path, FALSE, -1,
>> +                                 local_abspath, FALSE, -1,
>>                                  cancel_func, cancel_baton,
>> -                                 pool));
>> +                                 scratch_pool));
>
> Access batons should not be opened with absolute paths. The
> association stuff will not work properly.
>
> I also suspect you want to use one of the new "open with DB" functions
> that Bert added. And if you do, then using consistent paths will be
> even MORE important.
>
> Of course, better yet is to just remove access baton usage from this function.

Even better is to revamp the status stuff and entirely remove the guts
of this function. The callback is a simple function. Using editors in
here is an insane way to do a simple walk to find nodes to report
status on.

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2377742
Received on 2009-08-01 15:04:33 CEST

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