brane@tigris.org writes:
> --- trunk/subversion/libsvn_client/simple_providers.c (original)
> +++ trunk/subversion/libsvn_client/simple_providers.c Mon Apr 4 03:14:12 2005
> @@ -64,13 +67,22 @@
>
>
>
> +/* A function that takes a password IN, mangles it as per spec, and
> + returns it in OUT. The mangled password may be allocated in
> + POOL. */
> +typedef svn_boolean_t (*password_mangler_t) (const char **out, const char *in,
> + apr_pool_t *pool);
If *OUT is not allocated in POOL, then may we assume its lifetime is
permanent (i.e., it was allocated with malloc, or in its own one-time
pool)?
Oh, no, I see now. Looking at simple_password_mangler() later, it's
clear that *OUT could be the same as IN... Do you mind if I change the
doc string to say this:
/* A function that takes a password IN, mangles it as per spec, and
returns the mangled password in *OUT. *OUT is either allocated
in POOL, or is the same as IN. */
? That way there are no lifetime questions.
> +static svn_boolean_t
> +simple_password_mangler (const char **out, const char *in, apr_pool_t *pool)
> +{
> + (void)pool; /* Silence compiler warnings */
> + *out = in;
> + return TRUE;
> +}
> +
> +static svn_error_t *
> +simple_first_creds (void **credentials,
> + void **iter_baton,
> + void *provider_baton,
> + apr_hash_t *parameters,
> + const char *realmstring,
> + apr_pool_t *pool)
> +{
> + return simple_first_creds_helper (credentials,
> + iter_baton, provider_baton,
> + parameters, realmstring,
> + simple_password_mangler,
> + SVN_CLIENT__SIMPLE_PASSWORD_TYPE,
> + pool);
> +}
> +
> +static svn_error_t *
> +simple_save_creds (svn_boolean_t *saved,
> + void *credentials,
> + void *provider_baton,
> + apr_hash_t *parameters,
> + const char *realmstring,
> + apr_pool_t *pool)
> +{
> + return simple_save_creds_helper (saved, credentials, provider_baton,
> + parameters, realmstring,
> + simple_password_mangler,
> + SVN_CLIENT__SIMPLE_PASSWORD_TYPE,
> + pool);
> +}
I realize that a lot of the file is not documented, but it would be
nice to at least say what interfaces things implement.
> +static const WCHAR description[] = L"auth_svn.simple.wincrypt";
> +static const char windows_crypto_type[] = "wincrypt";
> +
> +static svn_boolean_t
> +get_crypto_function (const char *name, HINSTANCE *pdll, FARPROC *pfn)
> +{
> + HINSTANCE dll = LoadLibraryA ("Crypt32.dll");
> + if (dll)
> + {
> + FARPROC fn = GetProcAddress (dll, name);
> + if (fn)
> + {
> + *pdll = dll;
> + *pfn = fn;
> + return TRUE;
> + }
> + FreeLibrary (dll);
> + }
> + return FALSE;
> +}
Document?
> +static svn_boolean_t
> +windows_password_encrypter (const char **out, const char *in, apr_pool_t *pool)
> +{
Document, or at least say what interface is being implemented?
> +static svn_boolean_t
> +windows_password_decrypter (const char **out, const char *in, apr_pool_t *pool)
> +{
Same?
> +static svn_error_t *
> +windows_simple_first_creds (void **credentials,
> + void **iter_baton,
> + void *provider_baton,
> + apr_hash_t *parameters,
> + const char *realmstring,
> + apr_pool_t *pool)
> +{
> + return simple_first_creds_helper (credentials,
> + iter_baton, provider_baton,
> + parameters, realmstring,
> + windows_password_decrypter,
> + SVN_CLIENT__WINCRYPT_PASSWORD_TYPE,
> + pool);
> +}
Same?
> +static svn_error_t *
> +windows_simple_save_creds (svn_boolean_t *saved,
> + void *credentials,
> + void *provider_baton,
> + apr_hash_t *parameters,
> + const char *realmstring,
> + apr_pool_t *pool)
> +{
> + return simple_save_creds_helper (saved, credentials, provider_baton,
> + parameters, realmstring,
> + windows_password_encrypter,
> + SVN_CLIENT__WINCRYPT_PASSWORD_TYPE,
> + pool);
> +}
Same?
Again, I know you didn't introduce the problem in this file, but we
could at least not perpetuate it.
I didn't see any problems in the code itself.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Apr 4 17:43:40 2005