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

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.

Thanks, Dan.

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