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

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

From: Branko ─îibej <brane_at_xbc.nu>
Date: Tue, 20 May 2008 11:32:23 +0200

Philip Martin wrote:
> 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?
>
> Why is there no version checking like that in the ra and fs loaders?
>

I have to agree with this. Fix it, please.

-- Brane

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-20 11:33:00 CEST

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