Re: svn commit: r31291 - trunk/subversion/bindings/javahl/native

2008-05-19 20:24:13 Philip Martin napisał(a):
> arfrever_at_tigris.org writes:
>
> > Author: arfrever
> > Date: Sun May 18 12:40:54 2008
> > New Revision: 31291
> >
> > Log:
> > Use Keychain and KWallet in JavaHL.
> >
> > * subversion/libsvn_subr/cmdline.c
> > (get_auth_simple_provider): Copy from here ...
> > * subversion/bindings/javahl/native/SVNClient.cpp
> > (get_auth_simple_provider): ... to here.
> >
> > * subversion/bindings/javahl/native/SVNClient.cpp
> > (SVNClient::getContext): Use Keychain and KWallet providers.
> >
> > Modified:
> > trunk/subversion/bindings/javahl/native/SVNClient.cpp
> >
> > Modified: trunk/subversion/bindings/javahl/native/SVNClient.cpp
> > URL: http://svn.collab.net/viewvc/svn/trunk/subversion/bindings/javahl/native/SVNClient.cpp?pathrev=31291&r1=31290&r2=31291
> > ==============================================================================
> > --- trunk/subversion/bindings/javahl/native/SVNClient.cpp Sun May 18 12:31:12 2008 (r31290)
> > +++ trunk/subversion/bindings/javahl/native/SVNClient.cpp Sun May 18 12:40:54 2008 (r31291)
> > @@ -45,6 +45,7 @@
> > #include "StringArray.h"
> > #include "RevpropTable.h"
> > #include "svn_auth.h"
> > +#include "svn_dso.h"
> > #include "svn_types.h"
> > #include "svn_client.h"
> > #include "svn_sorts.h"
> > @@ -1151,6 +1152,40 @@ SVNClient::diffSummarize(const char *tar
> > requestPool.pool()), );
> > }
> >
> > +#if defined(SVN_HAVE_KWALLET) || defined(SVN_HAVE_GNOME_KEYRING)
> > +/* Dynamically load authentication simple provider. */
> > +static svn_boolean_t
> > +get_auth_simple_provider(svn_auth_provider_object_t **provider,
> > + const char *provider_name,
> > + apr_pool_t *pool)
> > +{
> > + apr_dso_handle_t *dso;
> > + apr_dso_handle_sym_t provider_symbol;
> > + const char *libname;
> > + const char *funcname;
> > + svn_boolean_t ret = FALSE;
> > + libname = apr_psprintf(pool,
> > + "libsvn_auth_%s-%d.so.0",
> > + provider_name,
> > + SVN_VER_MAJOR);
> > + funcname = apr_psprintf(pool,
> > + "svn_auth_get_%s_simple_provider",
> > + provider_name);
> > + svn_error_clear(svn_dso_load(&dso, libname));
> > + if (dso)
> > + {
> > + if (! apr_dso_sym(&provider_symbol, dso, funcname))
> > + {
> > + svn_auth_simple_provider_func_t func;
> > + func = (svn_auth_simple_provider_func_t) provider_symbol;
> > + func(provider, pool);
> > + ret = TRUE;
> > + }
> > + }
> > + return ret;
> > +}
> > +#endif
>
> I've seen this code before...
>
> Why is it acceptable to ignore errors from svn_dso_load?
>
> Why is it acceptable to use the dso variable if svn_dso_load returns
> an error?

Fixed in r31367.

> Why is there no version checking like that in the ra and fs loaders?

Patches welcome.
(I will work on the patch when I have time.)

-- 
Arfrever Frehtes Taifersar Arahesis