Re: svn commit: r1099955 - /subversion/trunk/subversion/libsvn_wc/status.c

[Greg Stein <gs...@gmail.com>]

On Thu, May 5, 2011 at 16:22,  <rh...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/status.c Thu May  5 20:22:43 2011
>...
> @@ -1132,34 +1134,31 @@ get_dir_status(const struct walk_status_
>               && info->status != svn_wc__db_status_excluded
>               && info->status != svn_wc__db_status_absent)
>             {
> -              svn_depth_t dir_depth;
> -              if (depth == svn_depth_files && info->kind == svn_wc__db_kind_dir)
> -                continue;
> -
> -              /* Handle this entry (possibly recursing). */
> -              dir_depth = (depth == svn_depth_infinity) ? depth
> -                                                        : svn_depth_empty;

Right here, dir_depth exits with: depth_infinity, or depth_empty.

> +              if (depth == svn_depth_files
> +                  && info->kind == svn_wc__db_kind_dir)
> +                {
> +                  continue;
> +                }
>
>               SVN_ERR(send_status_structure(wb, node_abspath,
> -                                                  dir_repos_root_url,
> -                                                  dir_repos_relpath,
> -                                                  info, dirent_p, get_all,
> -                                                  status_func, status_baton,
> -                                                  iterpool));
> -
> -              /* Descend only if the subdirectory is a working copy directory
> -                 and if DEPTH permits it.  */
> -              if ((info->kind == svn_wc__db_kind_dir)
> -                  && ((dir_depth == svn_depth_unknown
> -                       || dir_depth >= svn_depth_immediates)))

This condition only fires for dir_depth==depth_infinity (and not for
its only other possible value of empty). Thus: it fires only when
depth==depth_infinity.

> +                                            dir_repos_root_url,
> +                                            dir_repos_relpath,
> +                                            info, dirent_p, get_all,
> +                                            status_func, status_baton,
> +                                            iterpool));
> +
> +              if (depth == svn_depth_immediates)
> +                continue;

So then what is this all about? I laid this out in my original email:
you have depth_infinity or "everything else". The recursion should
only happen for depth_infinity. Your test above allows it to file for
depth_empty (or if not, then that is completely unknowable; I couldn't
find it). It doesn't fire for depth_files because that *happened* to
have been filtered out several lines above.

In short: this test is totally obscure. And there are no comments to explain.

> +
> +              /* Descend in subdirectories. */
> +              if (info->kind == svn_wc__db_kind_dir)

Why not just a simple && depth == infinity ?? Why a separate test and
continue? Way too many if's and branching and weird control flow in
here.

>...

Cheers,
-g

RE: svn commit: r1099955 - /subversion/trunk/subversion/libsvn_wc/status.c

[Bert Huijben <be...@qqmail.nl>]

> -----Original Message-----
> From: Greg Stein [mailto:gstein@gmail.com]
> Sent: vrijdag 6 mei 2011 0:09
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r1099955 -
> /subversion/trunk/subversion/libsvn_wc/status.c
> 

> In short: this test is totally obscure. And there are no comments to
explain.
> 
> > +
> > +              /* Descend in subdirectories. */
> > +              if (info->kind == svn_wc__db_kind_dir)
> 
> Why not just a simple && depth == infinity ?? Why a separate test and
> continue? Way too many if's and branching and weird control flow in
> here.

All the other depth values are handled above this block in this function.
Depth unknown is turned into infinity.

Depth empty doesn't get in the loop at all

Depth files is filtered before this block, for dirs.

And depth immediates is filtered here.

So that leaves depth infinity


The had_props flag was a 'hint' to skip reading the properties. The constant
TRUE values was cheaper then obtaining a real value in those callers. But
for that info instance we already get the real value.

	Bert