brainy commented on code in PR #32:
URL: https://github.com/apache/subversion/pull/32#discussion_r2178751023
##########
subversion/libsvn_ra_serf/util.c:
##########
@@ -958,7 +982,14 @@ svn_ra_serf__context_run(svn_ra_serf__session_t *sess,
_("Error running context"));
}
- return svn_ra_serf__wrap_err(status, _("Error running context"));
+ if (sess->ssl_error)
+ {
+ return sess->ssl_error;
+ }
+ else
+ {
+ return svn_ra_serf__wrap_err(status, _("Error running context"));
+ }
Review Comment:
`sess->ssl_error` should be set to NULL before returning. Even if `sess` is
discarded, it doesn't hurt to be future safe.
It also strikes me that the `if` here is not necessary. Instead,
`svn_ra_serf__wrap_err()` should take a new parameter for the child error, and
wrap the whole chain. Something like this, in `util_error.c`:
```
--- util_error.c (revision 1926861)
+++ util_error.c (working copy)
@@ -44,6 +44,7 @@
svn_error_t *
svn_ra_serf__wrap_err(apr_status_t status,
+ svn_error_t *child,
const char *fmt,
...)
{
@@ -51,7 +52,7 @@ svn_ra_serf__wrap_err(apr_status_t status,
svn_error_t *err;
va_list ap;
- err = svn_error_create(status, NULL, NULL);
+ err = svn_error_create(status, child, NULL);
if (serf_err_msg || fmt)
{
```
then you can just call svn_ra_serf__wrap_error(status, sess->ssl_error, ...`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@subversion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
