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

Re: svn commit: r27741 - in trunk/subversion: include libsvn_fs libsvn_fs_base libsvn_fs_fs tests/libsvn_fs

From: C. Michael Pilato <cmpilato_at_collab.net>
Date: 2007-11-09 22:51:43 CET

C. Michael Pilato wrote:
> Daniel L. Rall wrote:
>> On Fri, 09 Nov 2007, cmpilato@tigris.org wrote:
>> ...
>>> +static svn_error_t *
>>> +base_node_origin_rev(svn_revnum_t *revision,
>>> + svn_fs_root_t *root,
>>> + const char *path,
>>> + apr_pool_t *pool)
>>> +{
>>> + svn_fs_t *fs = svn_fs_root_fs(root);
>>> + svn_fs_root_t *curroot = root;
>>> + apr_pool_t *subpool = svn_pool_create(pool);
>>> + svn_stringbuf_t *lastpath =
>>> + svn_stringbuf_create(svn_fs__canonicalize_abspath(path, pool), pool);
>>> + svn_revnum_t lastrev = SVN_INVALID_REVNUM;
>>> + const svn_fs_id_t *pred_id;
>>> + struct id_created_rev_args icr_args;
>>> + svn_boolean_t found_copies = FALSE;
>>> +
>>> + /* Walk the closest-copy chain back to the first copy in our history.
>>> +
>>> + NOTE: We merely *assume* that this is faster than walking the
>>> + predecessor chain, because we *assume* that copies of parent
>>> + directories happen less often than modifications to a given item. */
>>> + while (1)
>>> + {
>>> + svn_revnum_t currev;
>>> + const char *curpath = lastpath->data;
>>> +
>>> + /* Find the previous location of this object using the
>>> + closest-copy shortcut. */
>>> + SVN_ERR(prev_location(&curpath, &currev, fs, curroot, curpath, subpool));
>>> + if (! curpath)
>>> + break;
>>> +
>>> + /* Update our LASTPATH and LASTREV variables (which survive SUBPOOL). */
>>> + found_copies = TRUE;
>>> + svn_stringbuf_set(lastpath, curpath);
>>> + lastrev = currev;
>>> +
>>> + /* Update our CURROOT from our calculated LASTREV. */
>>> + svn_pool_clear(subpool);
>> Any reason not to clear the subpool at the beginning of the loop (as per
>> conventions)?
>
> There absolutely *was* a reason in one phase of my coding (I even had a
> comment pointing out that intentional break from convention). I'll review
> to see if I can return to the convention without breaking anything.

Ah, yes. Because if I clear the subpool at the top, I kill the
svn_fs_root_t *curroot I need. But a little code rejuggling remedies that
*and* an extra root-fetch that occurs later. Thanks for the review -- the
code only gets better this way!

-- 
C. Michael Pilato <cmpilato@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

Received on Fri Nov 9 22:52:01 2007

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.