You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Hyrum K. Wright" <hy...@mail.utexas.edu> on 2008/03/01 22:37:16 UTC

Re: svn commit: r29663 - trunk/subversion/svnserve

glasser@tigris.org wrote:
> Author: glasser
> Date: Sat Mar  1 11:04:42 2008
> New Revision: 29663
> 
> Log:
> Followup to r29659: *really* fix a bunch of error leaks in the
> svnserve Cyrus SASL implementation.
> 
> * subversion/svnserve/cyrus_auth.c
>   (write_failure): New wrapper around svn_ra_svn_write_cmd_failure
>    which clears its error argument.
>   (fail_cmd, cyrus_auth_request): Use write_failure.
> 
> Found by: dionisos
> 
> 
> Modified:
>    trunk/subversion/svnserve/cyrus_auth.c
> 
> Modified: trunk/subversion/svnserve/cyrus_auth.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svnserve/cyrus_auth.c?pathrev=29663&r1=29662&r2=29663
> ==============================================================================
> --- trunk/subversion/svnserve/cyrus_auth.c	(original)
> +++ trunk/subversion/svnserve/cyrus_auth.c	Sat Mar  1 11:04:42 2008
> @@ -132,14 +132,24 @@
>    return svn_ra_svn_flush(conn, pool);
>  }
>  
> +/* Like svn_ra_svn_write_cmd_failure, but also clears the given error
> +   and sets it to SVN_NO_ERROR. */
> +static svn_error_t *
> +write_failure(svn_ra_svn_conn_t *conn, apr_pool_t *pool, svn_error_t **err_p)
> +{
> +  svn_error_t *write_err = svn_ra_svn_write_cmd_failure(conn, pool, *err);
> +  svn_error_clear(*err);
> +  *err = SVN_NO_ERROR;

subversion/svnserve/cyrus_auth.c: In function ‘write_failure’:
subversion/svnserve/cyrus_auth.c:140: error: ‘err’ undeclared (first use
in this function)

Did you mean err_p?

> +  return write_err;
> +}
> +
>  /* Used if we run into a SASL error outside try_auth(). */
>  static svn_error_t *
>  fail_cmd(svn_ra_svn_conn_t *conn, apr_pool_t *pool, sasl_conn_t *sasl_ctx)
>  {
>    svn_error_t *err = svn_error_create(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
>                                        sasl_errdetail(sasl_ctx));
> -  SVN_ERR(svn_ra_svn_write_cmd_failure(conn, pool, err));
> -  svn_error_clear(err);
> +  SVN_ERR(write_failure(conn, pool, &err));
>    return svn_ra_svn_flush(conn, pool);
>  }
>  
> @@ -242,8 +252,7 @@
>    if (apr_err)
>      {
>        svn_error_t *err = svn_error_wrap_apr(apr_err, _("Can't get hostname"));
> -      SVN_ERR(svn_ra_svn_write_cmd_failure(conn, pool, err));
> -      svn_error_clear(err);
> +      SVN_ERR(write_failure(conn, pool, &err));
>        return svn_ra_svn_flush(conn, pool);
>      }
>  
> @@ -258,8 +267,7 @@
>      {
>        svn_error_t *err = svn_error_create(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
>                                            sasl_errstring(result, NULL, NULL));
> -      SVN_ERR(svn_ra_svn_write_cmd_failure(conn, pool, err));
> -      svn_error_clear(err);
> +      SVN_ERR(write_failure(conn, pool, &err));
>        return svn_ra_svn_flush(conn, pool);
>      }
>  
> @@ -313,8 +321,7 @@
>        svn_error_t *err = svn_error_create(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
>                                            _("Could not obtain the list"
>                                            " of SASL mechanisms"));
> -      SVN_ERR(svn_ra_svn_write_cmd_failure(conn, pool, err));
> -      svn_error_clear(err);
> +      SVN_ERR(write_failure(conn, pool, &err));
>        return svn_ra_svn_flush(conn, pool);
>      }
>  
> @@ -354,8 +361,7 @@
>            err = svn_error_create(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
>                                   _("Couldn't obtain the authenticated"
>                                   " username"));
> -          SVN_ERR(svn_ra_svn_write_cmd_failure(conn, pool, err));
> -          svn_error_clear(err);
> +          SVN_ERR(write_failure(conn, pool, &err));
>            return svn_ra_svn_flush(conn, pool);
>          }
>      }
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
> 
> 



Re: svn commit: r29663 - trunk/subversion/svnserve

Posted by David Glasser <gl...@davidglasser.net>.
On Sat, Mar 1, 2008 at 2:51 PM, David Glasser <gl...@davidglasser.net> wrote:
>
> On Sat, Mar 1, 2008 at 2:37 PM, Hyrum K. Wright
>  <hy...@mail.utexas.edu> wrote:
>  >
>  > glasser@tigris.org wrote:
>  >  > Author: glasser
>  >  > Date: Sat Mar  1 11:04:42 2008
>  >  > New Revision: 29663
>  >  >
>  >  > Log:
>  >  > Followup to r29659: *really* fix a bunch of error leaks in the
>  >  > svnserve Cyrus SASL implementation.
>  >  >
>  >  > * subversion/svnserve/cyrus_auth.c
>  >  >   (write_failure): New wrapper around svn_ra_svn_write_cmd_failure
>  >  >    which clears its error argument.
>  >  >   (fail_cmd, cyrus_auth_request): Use write_failure.
>  >  >
>  >  > Found by: dionisos
>  >  >
>  >  >
>  >  > Modified:
>  >  >    trunk/subversion/svnserve/cyrus_auth.c
>  >  >
>  >  > Modified: trunk/subversion/svnserve/cyrus_auth.c
>  >  > URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svnserve/cyrus_auth.c?pathrev=29663&r1=29662&r2=29663
>  >  > ==============================================================================
>  >  > --- trunk/subversion/svnserve/cyrus_auth.c    (original)
>  >  > +++ trunk/subversion/svnserve/cyrus_auth.c    Sat Mar  1 11:04:42 2008
>  >  > @@ -132,14 +132,24 @@
>  >  >    return svn_ra_svn_flush(conn, pool);
>  >  >  }
>  >  >
>  >  > +/* Like svn_ra_svn_write_cmd_failure, but also clears the given error
>  >  > +   and sets it to SVN_NO_ERROR. */
>  >  > +static svn_error_t *
>  >  > +write_failure(svn_ra_svn_conn_t *conn, apr_pool_t *pool, svn_error_t **err_p)
>  >  > +{
>  >  > +  svn_error_t *write_err = svn_ra_svn_write_cmd_failure(conn, pool, *err);
>  >  > +  svn_error_clear(*err);
>  >  > +  *err = SVN_NO_ERROR;
>  >
>  >  subversion/svnserve/cyrus_auth.c: In function 'write_failure':
>  >  subversion/svnserve/cyrus_auth.c:140: error: 'err' undeclared (first use
>  >  in this function)
>  >
>  >  Did you mean err_p?
>
>  Uh, right.  I did (and tested) the first change on my work computer
>  and the second on my laptop.  Oh, and my laptop didn't have SASL, so
>  the whole file was ifdefed out :-)  Fixing now.

r29664.

--dave

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r29663 - trunk/subversion/svnserve

Posted by David Glasser <gl...@davidglasser.net>.
On Sat, Mar 1, 2008 at 2:37 PM, Hyrum K. Wright
<hy...@mail.utexas.edu> wrote:
>
> glasser@tigris.org wrote:
>  > Author: glasser
>  > Date: Sat Mar  1 11:04:42 2008
>  > New Revision: 29663
>  >
>  > Log:
>  > Followup to r29659: *really* fix a bunch of error leaks in the
>  > svnserve Cyrus SASL implementation.
>  >
>  > * subversion/svnserve/cyrus_auth.c
>  >   (write_failure): New wrapper around svn_ra_svn_write_cmd_failure
>  >    which clears its error argument.
>  >   (fail_cmd, cyrus_auth_request): Use write_failure.
>  >
>  > Found by: dionisos
>  >
>  >
>  > Modified:
>  >    trunk/subversion/svnserve/cyrus_auth.c
>  >
>  > Modified: trunk/subversion/svnserve/cyrus_auth.c
>  > URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svnserve/cyrus_auth.c?pathrev=29663&r1=29662&r2=29663
>  > ==============================================================================
>  > --- trunk/subversion/svnserve/cyrus_auth.c    (original)
>  > +++ trunk/subversion/svnserve/cyrus_auth.c    Sat Mar  1 11:04:42 2008
>  > @@ -132,14 +132,24 @@
>  >    return svn_ra_svn_flush(conn, pool);
>  >  }
>  >
>  > +/* Like svn_ra_svn_write_cmd_failure, but also clears the given error
>  > +   and sets it to SVN_NO_ERROR. */
>  > +static svn_error_t *
>  > +write_failure(svn_ra_svn_conn_t *conn, apr_pool_t *pool, svn_error_t **err_p)
>  > +{
>  > +  svn_error_t *write_err = svn_ra_svn_write_cmd_failure(conn, pool, *err);
>  > +  svn_error_clear(*err);
>  > +  *err = SVN_NO_ERROR;
>
>  subversion/svnserve/cyrus_auth.c: In function 'write_failure':
>  subversion/svnserve/cyrus_auth.c:140: error: 'err' undeclared (first use
>  in this function)
>
>  Did you mean err_p?

Uh, right.  I did (and tested) the first change on my work computer
and the second on my laptop.  Oh, and my laptop didn't have SASL, so
the whole file was ifdefed out :-)  Fixing now.

--dave


>  > +  return write_err;
>  > +}
>  > +
>  >  /* Used if we run into a SASL error outside try_auth(). */
>  >  static svn_error_t *
>  >  fail_cmd(svn_ra_svn_conn_t *conn, apr_pool_t *pool, sasl_conn_t *sasl_ctx)
>  >  {
>  >    svn_error_t *err = svn_error_create(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
>  >                                        sasl_errdetail(sasl_ctx));
>  > -  SVN_ERR(svn_ra_svn_write_cmd_failure(conn, pool, err));
>  > -  svn_error_clear(err);
>  > +  SVN_ERR(write_failure(conn, pool, &err));
>  >    return svn_ra_svn_flush(conn, pool);
>  >  }
>  >
>  > @@ -242,8 +252,7 @@
>  >    if (apr_err)
>  >      {
>  >        svn_error_t *err = svn_error_wrap_apr(apr_err, _("Can't get hostname"));
>  > -      SVN_ERR(svn_ra_svn_write_cmd_failure(conn, pool, err));
>  > -      svn_error_clear(err);
>  > +      SVN_ERR(write_failure(conn, pool, &err));
>  >        return svn_ra_svn_flush(conn, pool);
>  >      }
>  >
>  > @@ -258,8 +267,7 @@
>  >      {
>  >        svn_error_t *err = svn_error_create(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
>  >                                            sasl_errstring(result, NULL, NULL));
>  > -      SVN_ERR(svn_ra_svn_write_cmd_failure(conn, pool, err));
>  > -      svn_error_clear(err);
>  > +      SVN_ERR(write_failure(conn, pool, &err));
>  >        return svn_ra_svn_flush(conn, pool);
>  >      }
>  >
>  > @@ -313,8 +321,7 @@
>  >        svn_error_t *err = svn_error_create(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
>  >                                            _("Could not obtain the list"
>  >                                            " of SASL mechanisms"));
>  > -      SVN_ERR(svn_ra_svn_write_cmd_failure(conn, pool, err));
>  > -      svn_error_clear(err);
>  > +      SVN_ERR(write_failure(conn, pool, &err));
>  >        return svn_ra_svn_flush(conn, pool);
>  >      }
>  >
>  > @@ -354,8 +361,7 @@
>  >            err = svn_error_create(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
>  >                                   _("Couldn't obtain the authenticated"
>  >                                   " username"));
>  > -          SVN_ERR(svn_ra_svn_write_cmd_failure(conn, pool, err));
>  > -          svn_error_clear(err);
>  > +          SVN_ERR(write_failure(conn, pool, &err));
>  >            return svn_ra_svn_flush(conn, pool);
>  >          }
>  >      }
>  >
>  > ---------------------------------------------------------------------
>  > To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
>  > For additional commands, e-mail: svn-help@subversion.tigris.org
>  >
>  >
>
>
>



-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org