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

Re: svn commit: r26533 - in trunk/subversion: include libsvn_client libsvn_subr libsvn_wc

From: Erik Huelsmann <ehuels_at_gmail.com>
Date: 2007-09-12 10:11:42 CEST

On 9/12/07, Karl Fogel <kfogel@red-bean.com> wrote:
> dionisos@tigris.org writes:
> > Log:
> > Create iterator drivers
> > (with some code cleanup and pool use fixes to show their value).
>
> Yay!
>
> > --- (empty file)
> > +++ trunk/subversion/include/svn_iter.h Mon Sep 10 23:24:39 2007
> > @@ -0,0 +1,124 @@
> >
> > [...]
> >
> > +/** Iterate over the elements in @a hash, calling @a func for each one until
> > + * there are no more elements or @a func returns an error.
> > + *
> > + * Uses @a pool for temporary allocations.
> > + *
> > + * On return - if @a func returns no errors - @a completed will be set
> > + * to @c TRUE.
> > + *
> > + * If @a func returns an error other than @c SVN_ERR_ITER_BREAK, that
> > + * error is returned. When @a func returns @c SVN_ERR_ITER_BREAK,
> > + * iteration is interrupted, but no error is returned and @a completed is
> > + * set to @c FALSE.
> > + *
> > + * @since New in 1.5.
> > + */
> > +svn_error_t *
> > +svn_iter_apr_hash(svn_boolean_t *completed,
> > + apr_hash_t *hash,
> > + svn_iter_apr_hash_cb_t func,
> > + void *baton,
> > + apr_pool_t *pool);
>
> Should be "*completed" in doc string (same for svn_iter_apr_array).
> Both fixed in r26542, np.
>
> > +#define SVN_ITER_BREAK SVN_NO_ERROR + 1
>
> What's this for?
>
> > +/** Helper macro to break looping in svn_iter_apr_array() and
> > + * svn_iter_apr_hash() driven loops.
> > + *
> > + * @note The error is just a means of communicating between
> > + * driver and callback. There is no need for it to exist
> > + * past the lifetime of the iterpool.
> > + *
> > + * @since New in 1.5.
> > + */
> > +#define svn_iter_break(pool) \
> > + do { \
> > + svn_error_t *tmp__err = apr_pcalloc((pool), sizeof(*tmp__err)); \
> > + tmp__err->apr_err = SVN_ERR_ITER_BREAK; \
> > + return tmp__err; \
> > + } while (0)
>
> Hmmm, an idea: could we just define a static error object here?
>
> svn_error_t svn_iter_shared_break = {
> SVN_ERR_ITER_BREAK, /* apr_status_t apr_err */
> NULL, /* const char *message */
> NULL, /* struct svn_error_t *child */
> NULL, /* apr_pool_t *pool */
> __FILE__, /* const char *file; */
> __LINE__ /* long line; */
> }

Yes, we could. Actually, that's what the SVN_ITER_BREAK is about; it's
a remnant of a discussion with dlr in IRC where I said something along
the lines of 'having callbacks return "(svn_error_t *)1" will
eliminate the need to create a real error object'. However we both
didn't feel to good about returning a non-NULL pointer without it
being a pointer to a real object.

> Then svn_iter.c could just use '&svn_iter_shared_break', and we'd
> avoid some overhead.

I'll need to look at how we solved this in svn_ctypes.h, but as far as
I recall there were problems with exporting variables from DLLs on
Windows, because supposedly MSVC needs to reference a pointer to the
variable whereas other compilers support manipulating the variable
directly, see http://svn.haxx.se/dev/archive-2005-12/0364.shtml

I guess returning "(svn_error_t *)1" is the most portable solution to
returning a constant-and-known-pointer value to check against...

Comments?

bye,

Erik.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Sep 12 10:08:27 2007

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.