You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@google.com> on 2006/08/22 06:47:53 UTC
Re: svn commit: r21140 - trunk/subversion/libsvn_ra_svn
rooneg@tigris.org writes:
> Log:
> Use pool cleanups to make sure sasl contexts are always destroyed.
>
> Patch by: Vlad Georgescu <vg...@gmail.com>
> Tweaked by: me
>
> * subversion/libsvn_ra_svn/sasl_auth.c
> (sasl_dispose_cb): New pool cleanup function.
> (new_sasl_ctx): Install the cleanup.
> (svn_ra_svn__do_auth): Don't call sasl_dispose.
>
> --- trunk/subversion/libsvn_ra_svn/sasl_auth.c (original)
> +++ trunk/subversion/libsvn_ra_svn/sasl_auth.c Mon Aug 21 07:59:45 2006
> @@ -162,6 +162,13 @@
> return SVN_NO_ERROR;
> }
>
> +static apr_status_t sasl_dispose_cb(void *data)
> +{
> + sasl_conn_t *sasl_ctx = data;
> + sasl_dispose(&sasl_ctx);
> + return APR_SUCCESS;
> +}
> +
> /* Create a new SASL context. */
> static svn_error_t *new_sasl_ctx(sasl_conn_t **sasl_ctx,
> svn_boolean_t is_tunneled,
> @@ -180,6 +187,8 @@
> return svn_error_create(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
> sasl_errstring(result, NULL, NULL));
>
> + apr_pool_cleanup_register(pool, *sasl_ctx, sasl_dispose_cb,
> + apr_pool_cleanup_null);
This strikes me as weird, but perhaps I'm worried about nothing:
The parameter 'sasl_ctx' comes into new_sasl_ctx() as:
sasl_conn_t **sasl_ctx
We register it for cleanup with a single dereferencing:
apr_pool_cleanup_register(pool, *sasl_ctx, sasl_dispose_cb,
apr_pool_cleanup_null);
Then when we clean it up, we do so like this:
static apr_status_t sasl_dispose_cb(void *data)
{
sasl_conn_t *sasl_ctx = data;
sasl_dispose(&sasl_ctx);
return APR_SUCCESS;
}
So we're passing the address of the *local* parameter sasl_ctx to
sasl_dispose(). Presumably, sasl_dispose() NULLs out the variable
itself after disposing of whatever interior context it contains, but
we never use that variable again anyway, so it doesn't matter for us.
Is what's going on here that, for our case, even though the NULLing
out of the variable is useless, we have to pass it that way simply to
satisfy sasl_dispose()'s prototype?
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r21140 - trunk/subversion/libsvn_ra_svn
Posted by Karl Fogel <kf...@google.com>.
"Vlad Georgescu" <vg...@gmail.com> writes:
> On 8/22/06, Karl Fogel <kf...@google.com> wrote:
>> So we're passing the address of the *local* parameter sasl_ctx to
>> sasl_dispose(). Presumably, sasl_dispose() NULLs out the variable
>> itself after disposing of whatever interior context it contains, but
>> we never use that variable again anyway, so it doesn't matter for us.
>> Is what's going on here that, for our case, even though the NULLing
>> out of the variable is useless, we have to pass it that way simply to
>> satisfy sasl_dispose()'s prototype?
>
> Yes, that's exactly what's going on.
Thank you.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r21140 - trunk/subversion/libsvn_ra_svn
Posted by Vlad Georgescu <vg...@gmail.com>.
On 8/22/06, Karl Fogel <kf...@google.com> wrote:
> This strikes me as weird, but perhaps I'm worried about nothing:
>
> The parameter 'sasl_ctx' comes into new_sasl_ctx() as:
>
> sasl_conn_t **sasl_ctx
>
> We register it for cleanup with a single dereferencing:
>
> apr_pool_cleanup_register(pool, *sasl_ctx, sasl_dispose_cb,
> apr_pool_cleanup_null);
>
> Then when we clean it up, we do so like this:
>
> static apr_status_t sasl_dispose_cb(void *data)
> {
> sasl_conn_t *sasl_ctx = data;
> sasl_dispose(&sasl_ctx);
> return APR_SUCCESS;
> }
>
> So we're passing the address of the *local* parameter sasl_ctx to
> sasl_dispose(). Presumably, sasl_dispose() NULLs out the variable
> itself after disposing of whatever interior context it contains, but
> we never use that variable again anyway, so it doesn't matter for us.
> Is what's going on here that, for our case, even though the NULLing
> out of the variable is useless, we have to pass it that way simply to
> satisfy sasl_dispose()'s prototype?
Yes, that's exactly what's going on.
--
Vlad
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org