[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r21140 - trunk/subversion/libsvn_ra_svn

From: Karl Fogel <kfogel_at_google.com>
Date: 2006-08-22 08:47:53 CEST

rooneg@tigris.org writes:
> Log:
> Use pool cleanups to make sure sasl contexts are always destroyed.
>
> Patch by: Vlad Georgescu <vgeorgescu@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
Received on Tue Aug 22 08:48:23 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.