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

Re: RFC: svn_iter_* iteration drivers for common iterations

From: Erik Huelsmann <ehuels_at_gmail.com>
Date: 2007-09-10 21:56:00 CEST

On 9/10/07, Karl Fogel <kfogel@red-bean.com> wrote:
> "Erik Huelsmann" <ehuels@gmail.com> writes:
> > macro:
> > #define svn_iter_break return svn_error_create(SVN_ERR_CANCELLED, NULL, NULL)
> >
> >
> > How does it work?
> > The iterator callbacks are called for each item in the set. (If the
> > set is ordered, we could consider adding an optional 'from_end'
> > boolean, to make it explicitly iterate forward or backward). If the
> > iteration is to be cancelled, the iterator callback returns
> > SVN_ERR_CANCELLED.
> >
> > Returning an error (including SVN_ERR_CANCELLED) from the iterator
> > callback acts like the C 'break;' statement:it stops the iteration.
> >
> > The drivers have a *completed boolean return value which is set to
> > TRUE if the iteration was completed successfully, or FALSE otherwise.
> >
> > The iterator driver return value is the one returned by the iterator
> > callback, except for SVN_ERR_CANCELLED in which case the driver
> > returns SVN_NO_ERROR.
>
> +1
>
> Maybe don't overload SVN_ERR_CANCELLED for this, but instead use a
> dedicated, self-documenting error code -- SVN_ERR_ITER_BREAK or
> something.

Good point: It might be returned while (accidentally) being in a loop.

> I'm a little worried about using an error object as the "stop" marker
> at all. When we have nested iterations, the inner iteration will end
> up generating lots of error objects. True, they will be cleared, but
> there's some overhead to just allocating something in the global pool.

> The only alternative I can think of would be that all iter '*_cb_t'
> functions would have a boolean '&stop' parameter, which they set to
> true when they want to break out of the iteration.

Well, to avoid tons of parameters, I decided to use an error return.
Also, current

  if (expression)
    break;

constructs can be easily rewritten to

  if (expression)
    svn_iter_break;

where svn_iter_break is a macro expanding to a return statement
returning the required error. It would be a lot harder to implement
the same thing when using a parameter because it can have different
names in different functions.

> (I don't have hard numbers to back up this worry, so perhaps I'm just
> being a curmudgeon.)

Well, it's definitely not the common case for most loops, but I can't
be absolutely sure it's not ever. My feeling is that it's not though.

> I found your rewritten code more readable, btw.

Good. I'll continue along these lines then.

Thanks for the review.

bye,

Erik.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Sep 10 21:52:37 2007

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