pburba@tigris.org writes:
> Author: pburba
> Date: Fri Aug 25 07:39:09 2006
> New Revision: 21259
> @@ -322,12 +322,42 @@
> /* Read the directory entries one by one and add those things to
> revision control. */
> SVN_ERR(svn_io_dir_open(&dir, dirname, pool));
> - for (err = svn_io_dir_read(&this_entry, flags, dir, subpool);
> - err == SVN_NO_ERROR;
> - err = svn_io_dir_read(&this_entry, flags, dir, subpool))
> +
> + while (1)
> {
> const char *fullpath;
>
> + /* Clean out the per-iteration pool. */
> + svn_pool_clear(subpool);
That's a standard Subversion idiom so adding an "obvious" comment like
that doesn't really help, if anything it obscures the code.
> +
> + err = svn_io_dir_read(&this_entry, flags, dir, subpool);
> +
> + if (err)
> + {
> + /* Check if we're done reading the dir's entries. */
> + if (APR_STATUS_IS_ENOENT(err->apr_err))
> + {
> + /* No more entries, close the dir and exit the loop. */
Stating the obvious: "close the dir and exit the loop" is pointless
and could well become outdated if the code is changed in the future.
There are two comments "Check if ..." and "No more ..." that basically
cover the same thing, the fact that ENOENT is the end of the loop.
> + apr_status_t apr_err;
> +
> + svn_error_clear(err);
> + apr_err = apr_dir_close(dir);
> + if (apr_err)
> + return svn_error_wrap_apr
> + (apr_err, _("Can't close directory '%s'"),
> + svn_path_local_style(dirname, subpool));
> + break;
> + }
> + else
> + {
> + /* Some unexpected error reading the dir's entries. */
That comment doesn't add anything.
> + return svn_error_createf
> + (err->apr_err, err,
> + _("Error during recursive add of '%s'"),
> + svn_path_local_style(dirname, subpool));
> + }
> + }
> +
> /* Skip entries for this dir and its parent. */
> if (this_entry.name[0] == '.'
> && (this_entry.name[1] == '\0'
> @@ -364,29 +394,6 @@
> else if (err)
> return err;
> }
> -
> - /* Clean out the per-iteration pool. */
> - svn_pool_clear(subpool);
> - }
> -
> - /* Check that the loop exited cleanly. */
> - if (! (APR_STATUS_IS_ENOENT(err->apr_err)))
> - {
> - return svn_error_createf
> - (err->apr_err, err,
> - _("Error during recursive add of '%s'"),
> - svn_path_local_style(dirname, subpool));
> - }
> - else /* Yes, it exited cleanly, so close the dir. */
> - {
> - apr_status_t apr_err;
> -
> - svn_error_clear(err);
> - apr_err = apr_dir_close(dir);
> - if (apr_err)
> - return svn_error_wrap_apr
> - (apr_err, _("Can't close directory '%s'"),
> - svn_path_local_style(dirname, subpool));
> }
>
> /* Opened by svn_wc_add */
>
> Modified: trunk/subversion/libsvn_wc/copy.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/copy.c?pathrev=21259&r1=21258&r2=21259
> ==============================================================================
> --- trunk/subversion/libsvn_wc/copy.c (original)
> +++ trunk/subversion/libsvn_wc/copy.c Fri Aug 25 07:39:09 2006
> @@ -161,17 +161,50 @@
> SVN_ERR(svn_wc_adm_retrieve(&src_child_dir_access, src_access,
> src_path, pool));
>
> + /* Read src_path's entries one by one. */
> + SVN_ERR(svn_io_dir_open(&dir, src_path, pool));
That comment appears to apply to the svn_io_dir_open line, but
dir_open doesn't read any entries.
> +
> /* Create a subpool for iterative memory control. */
> subpool = svn_pool_create(pool);
Not part of your patch, but another pointless comment (I suppose that
might explain why you added comments in the same style).
>
> - /* Read src_path's entries one by one. */
> - SVN_ERR(svn_io_dir_open(&dir, src_path, pool));
> - for (err = svn_io_dir_read(&this_entry, flags, dir, subpool);
> - err == SVN_NO_ERROR;
> - err = svn_io_dir_read(&this_entry, flags, dir, subpool))
> + while (1)
> {
> const char *src_fullpath;
>
> + /* Clean out the per-iteration pool. */
> + svn_pool_clear(subpool);
Again.
> +
> + err = svn_io_dir_read(&this_entry, flags, dir, subpool);
> +
> + if (err)
> + {
> + /* Check if we're done reading the dir's entries. */
> + if (APR_STATUS_IS_ENOENT(err->apr_err))
> + {
> + /* No more entries, close the dir and exit the loop. */
> + apr_status_t apr_err;
> +
> + svn_error_clear(err);
> + apr_err = apr_dir_close(dir);
> + if (apr_err)
> + return svn_error_wrap_apr(apr_err,
> + _("Can't close "
> + "directory '%s'"),
> + svn_path_local_style(src_path,
> + subpool));
> + break;
> + }
> + else
> + {
> + /* Some unexpected error reading the dir's entries. */
> + return svn_error_createf(err->apr_err, err,
> + _("Error during recursive copy "
> + "of '%s'"),
> + svn_path_local_style(src_path,
> + subpool));
> + }
> + }
> +
> /* Skip entries for this dir and its parent. */
> if (this_entry.name[0] == '.'
> && (this_entry.name[1] == '\0'
> @@ -221,31 +254,9 @@
> subpool));
> }
>
> - /* Clean out the per-iteration pool. */
> - svn_pool_clear(subpool);
> -
> - } /* End for loop */
> + } /* End while(1) loop */
>
> - /* Check that the loop exited cleanly. */
> - if (! (APR_STATUS_IS_ENOENT(err->apr_err)))
> - {
> - return svn_error_createf(err->apr_err, err,
> - _("Error during recursive copy of '%s'"),
> - svn_path_local_style(src_path,
> - subpool));
> - }
> - else /* Yes, it exited cleanly, so close the dir. */
> - {
> - apr_status_t apr_err;
> -
> - svn_error_clear(err);
> - apr_err = apr_dir_close(dir);
> - if (apr_err)
> - return svn_error_wrap_apr(apr_err,
> - _("Can't close directory '%s'"),
> - svn_path_local_style(src_path,
> - subpool));
> - }
> + svn_pool_destroy(subpool);
>
> } /* End else src_is_added. */
>
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Aug 26 15:48:45 2006