You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@lyra.org> on 2006/03/31 01:27:44 UTC
Re: svn commit: r19108 - trunk/subversion/libsvn_ra_serf
On Thu, Mar 30, 2006 at 05:08:59PM -0800, jerenkrantz@tigris.org wrote:
>...
> +++ trunk/subversion/libsvn_ra_serf/update.c Thu Mar 30 17:08:59 2006
> @@ -1983,7 +1983,11 @@
> }
> if (status)
> {
> - return svn_error_wrap_apr(status, _("Error retrieving REPORT"));
> + if (sess->pending_error)
> + return sess->pending_error;
This could be written more simply as:
SVN_ERR(sess->pending_error);
Whether that is *clearer* ... dunno :-P
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r19108 - trunk/subversion/libsvn_ra_serf
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
Daniel Rall writes:
> On Fri, 31 Mar 2006, Garrett Rooney wrote:
>
> > On 3/31/06, Justin Erenkrantz <ju...@erenkrantz.com> wrote:
> > > On 3/31/06, Daniel Rall <dl...@collab.net> wrote:
> > > > I prefer Greg's suggested form on the grounds that it seems more
> > > > consistent with error handling in the rest of Subversion's source.
> > >
> > > It's a variable not a function. That's why it seems, well, wrong.
> >
> > I've seen that idiom (wrapping a variable in SVN_ERR) used in other
> > places in the svn code. It's not overly common though.
>
> Based on the macro's doc string, I can see where Justin's reaction was
> coming from.
>
> However, there's really little reason to avoid use of SVN_ERR(expr)
> when expr is not a function call. How about something like this?:
>
+1. I prefer this use in some context, if nothing else, just because a
one-line should be a one-line;) OTOH, in a series of if statements:
if (err && err->apr == ...)
...
else if (...)
...
else if (err)
return err;
I think the explicit use is easier to read.
Just my 2 öre (Swedish counterpart to cent)
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r19108 - trunk/subversion/libsvn_ra_serf
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 3/31/06, Daniel Rall <dl...@collab.net> wrote:
> Based on the macro's doc string, I can see where Justin's reaction was
> coming from.
>
> However, there's really little reason to avoid use of SVN_ERR(expr)
> when expr is not a function call. How about something like this?:
Makes sense to me.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r19108 - trunk/subversion/libsvn_ra_serf
Posted by Daniel Rall <dl...@collab.net>.
On Fri, 31 Mar 2006, Garrett Rooney wrote:
> On 3/31/06, Justin Erenkrantz <ju...@erenkrantz.com> wrote:
> > On 3/31/06, Daniel Rall <dl...@collab.net> wrote:
> > > I prefer Greg's suggested form on the grounds that it seems more
> > > consistent with error handling in the rest of Subversion's source.
> >
> > It's a variable not a function. That's why it seems, well, wrong.
>
> I've seen that idiom (wrapping a variable in SVN_ERR) used in other
> places in the svn code. It's not overly common though.
Based on the macro's doc string, I can see where Justin's reaction was
coming from.
However, there's really little reason to avoid use of SVN_ERR(expr)
when expr is not a function call. How about something like this?:
[[[
* src/subversion/subversion/include/svn_error.h
(SVN_ERR): Tweak macro doc string to avoid making it seem that it
should only be used when evaluating return value of a function,
since it can also be used when evaluating variable.s
]]]
Index: src/subversion/subversion/include/svn_error.h
===================================================================
--- src/subversion/subversion/include/svn_error.h (revision 19113)
+++ src/subversion/subversion/include/svn_error.h (working copy)
@@ -205,7 +205,7 @@
void svn_handle_warning(FILE *stream, svn_error_t *error);
-/** A statement macro for checking error return values.
+/** A statement macro for checking error values.
*
* Evaluate @a expr. If it yields an error, return that error from the
* current function. Otherwise, continue.
Re: svn commit: r19108 - trunk/subversion/libsvn_ra_serf
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 3/31/06, Justin Erenkrantz <ju...@erenkrantz.com> wrote:
> On 3/31/06, Daniel Rall <dl...@collab.net> wrote:
> > I prefer Greg's suggested form on the grounds that it seems more
> > consistent with error handling in the rest of Subversion's source.
>
> It's a variable not a function. That's why it seems, well, wrong.
I've seen that idiom (wrapping a variable in SVN_ERR) used in other
places in the svn code. It's not overly common though.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r19108 - trunk/subversion/libsvn_ra_serf
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On 3/31/06, Daniel Rall <dl...@collab.net> wrote:
> I prefer Greg's suggested form on the grounds that it seems more
> consistent with error handling in the rest of Subversion's source.
It's a variable not a function. That's why it seems, well, wrong. -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r19108 - trunk/subversion/libsvn_ra_serf
Posted by Daniel Rall <dl...@collab.net>.
On Thu, 30 Mar 2006, Greg Stein wrote:
> On Thu, Mar 30, 2006 at 05:08:59PM -0800, jerenkrantz@tigris.org wrote:
> >...
> > +++ trunk/subversion/libsvn_ra_serf/update.c Thu Mar 30 17:08:59 2006
> > @@ -1983,7 +1983,11 @@
> > }
> > if (status)
> > {
> > - return svn_error_wrap_apr(status, _("Error retrieving REPORT"));
> > + if (sess->pending_error)
> > + return sess->pending_error;
>
> This could be written more simply as:
>
> SVN_ERR(sess->pending_error);
>
> Whether that is *clearer* ... dunno :-P
I prefer Greg's suggested form on the grounds that it seems more
consistent with error handling in the rest of Subversion's source.
--
Daniel Rall