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

Re: [PATCH] v2. replace ALL svn_wc_entry() in libsvn_client/patch.c

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Tue, 18 Aug 2009 18:12:26 +0100

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.

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2384858
Received on 2009-08-18 19:12:45 CEST

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