On Aug 18, 2009, at 12:12 PM, Julian Foad wrote:
> Daniel Näslund wrote:
>> [[[
>> Replaced svn_wc_entry() with svn_wc__get_entry_versioned(). Catch
>> SVN_ERR_ENTRY_NOT_FOUND since it is not expected.
>
> I know you know why you're doing this, but tell us in the log message:
>
> As part of getting rid of adm_access batons for WC-NG, replace
> some calls to svn_wc_entry() with svn_wc__get_entry_versioned().
>
>
>> * subversion/libsvn_client/patch.c
>> (merge_file_changed, merge_file_added, merge_file_deleted,
>> merge_dir_added, init_patch_target): Replaced svn_wc_entry(). Used
>> absolute paths.
>> ]]]
> [...]
>> @@ -369,7 +391,26 @@
>> case svn_node_none:
>> {
>> const svn_wc_entry_t *entry;
>> - SVN_ERR(svn_wc_entry(&entry, mine, adm_access, FALSE,
>> subpool));
>> + svn_error_t *err;
>> +
>> + err = svn_wc__get_entry_versioned(&entry, patch_b->ctx-
>> >wc_ctx,
>> + mine_abspath,
>> svn_node_none,
>> + FALSE, FALSE,
>> + subpool, subpool);
>> +
>
> The indentation is off below here...
>
>> + /* We catch the error since a NULL value is expected
>> when the entry
>> + is hidden. */
>> + if (err && err->apr_err == SVN_ERR_ENTRY_NOT_FOUND)
>> + {
>> + svn_error_clear(err);
>> + entry = NULL;
>> + }
>> + else if (err)
>> + {
>> + svn_pool_destroy(subpool);
>> + return svn_error_return(err);
>> + }
>
> Replacing one line of code with a long block like this, eight times
> over, cannot be a good thing. Can you make a replacement function that
> does exactly what we need in one line, and so avoid this code bloat?
> Even a local helper function in this file would be an improvement,
> but I
> assume this same pattern will be repeated in many files so a new
> svn_wc__... function would be best.
My inner self is saying the same thing here.
svn_wc__get_entry_versioned() was intentioned to replace
svn_wc__entry_versioned(), and we've found a few places where we can
get away with using it in place of svn_wc_entry(), but a modified
helper would be nice at this point.
-Hyrum
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2385076
Received on 2009-08-19 04:18:35 CEST