On Fri, Jan 24, 2003 at 03:42:21PM -0600, sussman@tigris.org wrote:
...
+++ trunk/subversion/include/svn_auth.h Fri Jan 24 15:42:11 2003
@@ -26,6 +26,7 @@
#include svn_types.h
#include svn_wc.h
+#include svn_client.h
Auth is a lower system than the client library. Even lower than the wc
library. The auth system should not provide wc-related providers -- the WC
should provide that itself. The prompt thing is okay, but the definition of
a prompt function should be local to the auth library rather than the client
library. You'll notice that libsvn_client makes zero use of the prompt
function, except for a reference in the client auth baton (which is
probably, inherently bogus in the new design).
libsvn_auth should be considered a base library which others build on. I
don't think it should know about WC-related things or really even a whole
lot about other Subversion concepts (e.g. RA or client).
...
+++ trunk/subversion/libsvn_auth/simple_prompt_provider.c Fri Jan 24 15:42:13 2003
...
+#include svn_client.h
+#include svn_wc.h
+#include svn_auth.h
Why svn_wc.h in this file? I see no use of WC types/functions.
+/* Bikeshed: how many times we re-try prompting. */
+#define SVN_AUTH_SIMPLE_PROMPT_RETRY_LIMIT 2
The prompter probably should not define this, but take it as an argument to
the provider-fetch function.
...
+typedef struct
+{
+ /* the same callback from the main provider baton */
+ svn_client_prompt_t prompt_func;
+ void *prompt_baton;
Might be simpler to just have a back-pointer to the provider baton.
...
+{
+ simple_prompt_provider_baton_t *pb
+ = (simple_prompt_provider_baton_t *) provider_baton;
That cast doesn't do anything but make the line harder to read. Since
provider_baton is a 'void *', it is fine to assign it directly to pb.
+ svn_auth_cred_simple_t *creds = apr_pcalloc (pool, sizeof(*creds));
+ simple_prompt_iter_baton_t *ibaton = apr_pcalloc (pool, sizeof(*ibaton));
+ char *username, *password;
Why non-const? We don't intend to change these values, and it just means
that casts are needed later. I'd recommend altering the prototype on the
prompt function, rather than letting it's non-const return value monkey
stuff like this up.
...
+ creds = apr_pcalloc (pool, sizeof(*creds));
+
+ SVN_ERR (ib-prompt_func (username, username: ,
If the prompt_func type were fixed, you could just do: creds-username.
...
+/* The provider. */
+const svn_auth_provider_t simple_prompt_provider =
+ {
+ SVN_AUTH_CRED_SIMPLE, /* username/passwd creds */
+ simple_prompt_first_creds,
+ simple_prompt_next_creds,
+ NULL /* this provider can't save creds. */
+ };
This needs to be 'static const'; otherwise, it appears outside of the
link-unit, thus infecting the global namespace with something that doesn't
start with 'svn'.
+/* Public API */
+void
+svn_auth_get_simple_prompt_provider (const svn_auth_provider_t **provider,
+ void **provider_baton,
+ svn_client_prompt_t prompt_func,
+ void *prompt_baton,
+ const char *default_username,
+ const char *default_password,
+ apr_pool_t *pool)
+{
+ simple_prompt_provider_baton_t *pb = apr_pcalloc (pool, sizeof(*pb));
+ pb-prompt_func = prompt_func;
+ pb-prompt_baton = prompt_baton;
+ pb-default_username = apr_pstrdup (pool, default_username);
+ pb-default_password = apr_pstrdup (pool, default_password);
You aren't checking whether the defaults are NULL before dup'ing them.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 14 02:19:17 2006