On Sat, 08 Sep 2007, David Glasser wrote:
> On 9/6/07, dlr@tigris.org <dlr@tigris.org> wrote:
> > Author: dlr
> > Date: Thu Sep 6 10:09:35 2007
> > New Revision: 26483
> >
> > Log:
> > Formalize a flow control macro which we apparently use in several
> > places already.
> >
> > * subversion/include/private/svn_flowcontrol.h
> > New header file, providing a MAYBE_GOTO() macro.
>
> I'm not really sure how compelling this is: I don't think that
>
> MAYBE_GOTO(cleanup, err);
>
> is actually any clearer than
>
> if (err) goto cleanup;
>
> It's longer, makes the reader think a little more (since it isn't just
> standard C conditional), and doesn't really hide any complexity...
I had similar misgivings. I'm certainly not attached to it -- was
just cleaning out a WC...
> On the other hand, a macro that does SVN_ERR-style error variable
> allocation *would* be compelling to me (ie, a macro that saves the
> developer from having to define and name yet another svn_error_t*
> variable), but I'm not sure if that would work if the cleanup section
> needs to be able to refer to the error. Possibly we should just be
> ditching goto, factoring the "cleanup" sections into a new function,
> and writing
>
> MAYBE_CLEANUP(something_returning_error(), cleanup_func);
>
> which expands to
>
> do {
> svn_error_t *svn_error_tmp = (something_returning_error());
> if (svn_error_tmp)
> return cleanup_func(svn_error_tmp);
> } while (0);
>
> ? Except for you'd probably need some sort of context baton, so maybe not...
Yeah, the places where this pattern is used typically need access to
some resources (pools, DB connections, etc.) to perform the necessary
cleanup activities. You'd sometimes end up declaring and setting up
some local data types to make this feasible.
I'll revert this change for now.
- Dan
- application/pgp-signature attachment: stored
Received on Mon Sep 10 21:25:36 2007