You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@codematters.co.uk> on 2003/12/02 20:32:21 UTC
Re: svn commit: rev 7905 - trunk/subversion/libsvn_client
julianfoad@tigris.org writes:
> Author: julianfoad
> Date: Tue Dec 2 12:33:53 2003
> New Revision: 7905
>
> Modified:
> trunk/subversion/libsvn_client/commit.c
> Log:
> Simplify code in 'import_dir' by using a library function.
>
> * subversion/libsvn_client/commit.c (import_dir)
> Call svn_io_get_dirents instead of doing the hard work manually.
>
>
> Modified: trunk/subversion/libsvn_client/commit.c
> ==============================================================================
> --- trunk/subversion/libsvn_client/commit.c (original)
> +++ trunk/subversion/libsvn_client/commit.c Tue Dec 2 12:33:53 2003
> @@ -177,40 +177,34 @@
> apr_pool_t *pool)
> {
> apr_pool_t *subpool = svn_pool_create (pool); /* iteration pool */
> - apr_dir_t *dir;
> - apr_finfo_t finfo;
> - apr_status_t apr_err;
> + apr_hash_t *dirents;
> + apr_hash_index_t *hi;
> apr_int32_t flags = APR_FINFO_TYPE | APR_FINFO_NAME;
> - svn_error_t *err;
> apr_array_header_t *ignores;
>
> - SVN_ERR (svn_io_dir_open (&dir, path, pool));
> -
> SVN_ERR (svn_wc_get_default_ignores (&ignores, ctx->config, pool));
>
> - for (err = svn_io_dir_read (&finfo, flags, dir, subpool);
> - err == SVN_NO_ERROR;
> - svn_pool_clear (subpool),
> - err = svn_io_dir_read (&finfo, flags, dir, subpool))
> + SVN_ERR (svn_io_get_dirents (&dirents, path, pool));
> +
> + for (hi = apr_hash_first (pool, dirents);
> + hi;
> + svn_pool_clear (subpool), hi = apr_hash_next (hi))
I'd classify that as gratuitous use of the comma operator :)
Looking at the old code I see that it is using the subpool in
svn_io_dir_read, which probably explains why the comma got used
originally. Your code doesn't need it. Do people like this use of
the comma operator?
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 7905 - trunk/subversion/libsvn_client
Posted by John Szakmeister <jo...@szakmeister.net>.
On Tuesday 02 December 2003 15:32, Philip Martin wrote:
[snip]
> > + SVN_ERR (svn_io_get_dirents (&dirents, path, pool));
> > +
> > + for (hi = apr_hash_first (pool, dirents);
> > + hi;
> > + svn_pool_clear (subpool), hi = apr_hash_next (hi))
>
> I'd classify that as gratuitous use of the comma operator :)
>
> Looking at the old code I see that it is using the subpool in
> svn_io_dir_read, which probably explains why the comma got used
> originally. Your code doesn't need it. Do people like this use of
> the comma operator?
I personally don't like it. I'd rather the 'for' just be related to advancing
and checking the iterator itself. But, as long as we're consistent I don't
mind.
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 7905 - trunk/subversion/libsvn_client
Posted by Julian Foad <ju...@btopenworld.com>.
Philip Martin wrote:
> julianfoad@tigris.org writes:
>
>>- for (err = svn_io_dir_read (&finfo, flags, dir, subpool);
>>- err == SVN_NO_ERROR;
>>- svn_pool_clear (subpool),
>>- err = svn_io_dir_read (&finfo, flags, dir, subpool))
>>+ SVN_ERR (svn_io_get_dirents (&dirents, path, pool));
>>+
>>+ for (hi = apr_hash_first (pool, dirents);
>>+ hi;
>>+ svn_pool_clear (subpool), hi = apr_hash_next (hi))
>
> I'd classify that as gratuitous use of the comma operator :)
>
> Looking at the old code I see that it is using the subpool in
> svn_io_dir_read, which probably explains why the comma got used
> originally. Your code doesn't need it. Do people like this use of
> the comma operator?
I don't much like it, but I hadn't spotted the reason for it that you point out. Now that I know that, I am happy to remove the comma and move the pool clearing into the body of the loop.
Thanks for pointing it out.
I see now that I also left a variable that is now unused.
Fixed both in r7914.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org