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

Re: [PATCH] Remove unnecessary props read in svn_wc_get_prop_diffs()

From: Malcolm Rowe <malcolm-svn-dev_at_farside.org.uk>
Date: 2005-11-02 23:46:07 CET

On Wed, Nov 02, 2005 at 09:53:17PM +0000, Philip Martin wrote:
> > 2. We could also move the initialisation of the localprops local
> > variable further down, but I assume that the call to apr_hash_make()
> > is inexpensive.
>
> If you used the "big if" style it could go inside the if.
>

You're right. I moved local_propchanges as well.

> > + /* Load the BASE props. */
>
> I'm not a fan of "trivial" comments, ones that state what the code
> does rather than why, and I think that one comes into the trivial
> category.
>
> > + /* Load the WORKING props. */
>
> That one is also trivial.
>

Okay, agreed. I used them when I wasn't sure of what was going on,
but they don't impart anything useful now.

Updated patch attached. I also realised that the calls to
svn_wc__prop_path() and svn_wc__load_prop_file() to read the WORKING
properties file exactly duplicated the logic in svn_wc_props_list(),
so I replaced them with a call to that instead.

[[[
* subversion/libsvn_wc/props.c
  (svn_wc_get_prop_diffs): Rearrange, avoiding an unnecessary read of
    the WORKING properties file if the caller doesn't want to receive
    property changes. Replace calls to read the properties file with
    a single call to svn_wc_props_list().
]]]

Regards,
Malcolm

Index: subversion/libsvn_wc/props.c
===================================================================
--- subversion/libsvn_wc/props.c (revision 17151)
+++ subversion/libsvn_wc/props.c (working copy)
@@ -1799,33 +1799,32 @@ svn_error_t *
 svn_wc_get_prop_diffs (apr_array_header_t **propchanges,
                        apr_hash_t **original_props,
                        const char *path,
                        svn_wc_adm_access_t *adm_access,
                        apr_pool_t *pool)
 {
- const char *prop_path, *prop_base_path;
- apr_array_header_t *local_propchanges;
- apr_hash_t *localprops = apr_hash_make (pool);
+ const char *prop_base_path;
   apr_hash_t *baseprops = apr_hash_make (pool);
 
-
- SVN_ERR (svn_wc__prop_path (&prop_path, path, adm_access, FALSE, pool));
   SVN_ERR (svn_wc__prop_base_path (&prop_base_path, path, adm_access, FALSE,
                                    pool));
-
- SVN_ERR (svn_wc__load_prop_file (prop_path, localprops, pool));
   SVN_ERR (svn_wc__load_prop_file (prop_base_path, baseprops, pool));
 
   if (original_props != NULL)
     *original_props = baseprops;
 
- /* At this point, if either of the propfiles are non-existent, then
- the corresponding hash is simply empty. */
-
   if (propchanges != NULL)
     {
+ apr_hash_t *localprops;
+ apr_array_header_t *local_propchanges;
+
+ SVN_ERR (svn_wc_prop_list (&localprops, path, adm_access, pool));
+
+ /* At this point, if either of the propfiles are non-existent, then
+ the corresponding hash is simply empty. */
+
       SVN_ERR (svn_prop_diffs (&local_propchanges, localprops,
                                baseprops, pool));
       *propchanges = local_propchanges;
     }
 
   return SVN_NO_ERROR;

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Nov 2 23:46:51 2005

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