Re: svn commit: r37590 - trunk/subversion/libsvn_wc

On Tue, May 5, 2009 at 22:19, Hyrum K. Wright <hyrum_at_hyrumwright.org> wrote:
>...
> +++ trunk/subversion/libsvn_wc/adm_ops.c        Tue May  5 13:19:56 2009        (r37590)
>...
> @@ -2445,8 +2451,12 @@ svn_wc_remove_from_revision_control(svn_
>   svn_error_t *err;
>   svn_boolean_t is_file;
>   svn_boolean_t left_something = FALSE;
> +  svn_wc__db_t *db = svn_wc__adm_get_db(adm_access);
>   const char *full_path = apr_pstrdup(pool,
>                                       svn_wc_adm_access_path(adm_access));

I know this isn't part of your change, but this seems dumb. By
definition, the baton's path will last longer than this function. Is
there a reason to make a copy of this string?

> +  const char *local_abspath;
> +
> +  SVN_ERR(svn_path_get_absolute(&local_abspath, full_path, pool));
>
>   /* Check cancellation here, so recursive calls get checked early. */
>   if (cancel_func)
> @@ -2460,10 +2470,9 @@ svn_wc_remove_from_revision_control(svn_
>       svn_node_kind_t kind;
>       svn_boolean_t wc_special, local_special;
>       svn_boolean_t text_modified_p;
> -      svn_wc__db_t *db = svn_wc__adm_get_db(adm_access);
> -      const char *local_abspath;
>
>       full_path = svn_dirent_join(full_path, name, pool);
> +      SVN_ERR(svn_path_get_absolute(&local_abspath, full_path, pool));

You did this above.

>
>       /* Only check if the file was modified when it wasn't overwritten with a
>          special file */
> @@ -2482,16 +2491,15 @@ svn_wc_remove_from_revision_control(svn_
>         }
>
>       /* Remove the wcprops. */
> -      SVN_ERR(svn_wc__props_delete(full_path, svn_wc__props_wcprop,
> -                                   adm_access, pool));
> +      SVN_ERR(svn_wc__props_delete(db, local_abspath, svn_wc__props_wcprop,
> +                                   pool));
>       /* Remove prop/NAME, prop-base/NAME.svn-base. */
> -      SVN_ERR(svn_wc__props_delete(full_path, svn_wc__props_working,
> -                                   adm_access, pool));
> -      SVN_ERR(svn_wc__props_delete(full_path, svn_wc__props_base,
> -                                   adm_access, pool));
> +      SVN_ERR(svn_wc__props_delete(db, local_abspath, svn_wc__props_working,
> +                                   pool));
> +      SVN_ERR(svn_wc__props_delete(db, local_abspath, svn_wc__props_base,
> +                                   pool));
>
>       /* Remove NAME from PATH's entries file: */
> -      SVN_ERR(svn_path_get_absolute(&local_abspath, full_path, pool));
>       SVN_ERR(svn_wc__entry_remove(db, local_abspath, pool));
>
>       /* Remove text-base/NAME.svn-base */
> @@ -2534,8 +2542,8 @@ svn_wc_remove_from_revision_control(svn_
>       /* Get rid of all the wcprops in this directory.  This avoids rewriting
>          the wcprops file over and over (meaning O(n^2) complexity)
>          below. */
> -      SVN_ERR(svn_wc__props_delete(full_path, svn_wc__props_wcprop,
> -                                   adm_access, pool));
> +      SVN_ERR(svn_wc__props_delete(db, local_abspath, svn_wc__props_wcprop,
> +                                   pool));
>
>       /* Walk over every entry. */
>       SVN_ERR(svn_wc_entries_read(&entries, adm_access, FALSE, pool));
> @@ -2591,9 +2599,6 @@ svn_wc_remove_from_revision_control(svn_
>                   /* The directory is either missing or excluded,
>                      so don't try to recurse, just delete the
>                      entry in the parent directory. */
> -                  svn_wc__db_t *db = svn_wc__adm_get_db(adm_access);
> -                  const char *local_abspath;
> -
>                   SVN_ERR(svn_path_get_absolute(&local_abspath, entrypath,
>                                                 subpool));

Is it safe to ovewrite local_abspath? (I don't see all the context; is
it needed again?) Maybe call this entry_abspath instead?

>...

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2072542
Received on 2009-05-05 22:44:38 CEST