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

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

From: Greg Stein <gstein_at_gmail.com>
Date: Sat, 12 Sep 2009 05:11:03 -0400

On Fri, Sep 11, 2009 at 15:20, Hyrum K. Wright <hyrum_at_hyrumwright.org> wrote:
>...
> +++ trunk/subversion/libsvn_wc/update_editor.c  Fri Sep 11 12:20:46 2009        (r39251)
>...
> @@ -1435,27 +1435,34 @@ typedef struct modcheck_baton_t {
>  } modcheck_baton_t;
>
>  static svn_error_t *
> -modcheck_found_entry(const char *path,
> -                     const svn_wc_entry_t *entry,
> -                     void *walk_baton,
> -                     apr_pool_t *pool)
> +modcheck_found_node(const char *local_abspath,
> +                    void *walk_baton,
> +                    apr_pool_t *scratch_pool)
>  {
>   modcheck_baton_t *baton = walk_baton;
> +  svn_wc__db_kind_t kind;
> +  svn_wc__db_status_t status;
>   svn_boolean_t modified;
> -  const char *local_abspath;
>
> -  SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));
> +  SVN_ERR(svn_wc__db_read_info(&status, &kind, NULL, NULL, NULL, NULL, NULL,
> +                               NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> +                               NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> +                               NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> +                               baton->db, local_abspath, scratch_pool,
> +                               scratch_pool));
>
> -  if (entry->schedule != svn_wc_schedule_normal)
> +  if (status != svn_wc__db_status_normal)
>     modified = TRUE;
>   else
>     SVN_ERR(entry_has_local_mods(&modified, baton->db, local_abspath,
> -                                 entry->kind, pool));
> +                                 kind == svn_wc__db_kind_file
> +                                   ? svn_node_file : svn_node_dir,
> +                                 scratch_pool));

This is going to fail miserably if kind == svn_wc__db_kind_symlink.

I'd suggest leaving a ### comment about the problem (since we don't
actually use symlink yet), or testing for kind==dir and letting others
fall out as svn_node_fil (which a symlink is modeled as in wc-1).

>...
> @@ -1471,27 +1478,27 @@ modcheck_found_entry(const char *path,
>  static svn_error_t *
>  tree_has_local_mods(svn_boolean_t *modified,
>                     svn_boolean_t *all_edits_are_deletes,
> -                    const char *path,
> -                    svn_wc_adm_access_t *adm_access,
> +                    svn_wc__db_t *db,
> +                    const char *local_abspath,
>                     svn_cancel_func_t cancel_func,
>                     void *cancel_baton,
>                     apr_pool_t *pool)
>  {
> -  static const svn_wc_entry_callbacks2_t modcheck_callbacks =
> -    { modcheck_found_entry, svn_wc__walker_default_error_handler };
> +  static const svn_wc__node_walk_callbacks_t modcheck_callbacks =
> +    { modcheck_found_node, svn_wc__walker_default_error_handler };

I didn't review (yet) the new node walker, but an error callback is
bogus in the extreme. Thus, a struct of callbacks is wrong. Just pass
a simple function. *IF* a caller wants special error handling, then
they can do it *before* returning an error to the walker. There is NO
reason for the walker to be involved. I looked at this once, and
(iirc) the usage is a single occurrence. Totally bogus,
over-engineered walker blather. Please don't repeat it with the node
walker.

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2393823
Received on 2009-09-12 11:11:18 CEST

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