Greg Stein gstein@lyra.org writes:
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).
OK, I wasn't sure. I was under the impresion that libsvn_auth wasn't
at the bottom of the pyramid, but rather a long block along the side
of the pyramid... it could call any library, and any library could
call it.
But if you think it's better to put it at the bottom, we can do that.
It means the wc provider needs to live in libsvn_wc. No problem.
+/* 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.
Great idea!
+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.
I was thinking this too.
...
+{
+ 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.
Thanks, yah, I gotta get out of this overly verbose habit. :)
+ 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.
Indeed, the prompt_func prototype is horked. I didn't want to perturb
existing RA code. But now that I'm about to start integrating
libsvn_auth into existing code paths, I'll fix the prototype.
+/* 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'.
Doh! Of course. Thanks.
+/* 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.
Will fix.
Thank you, Herr Stein!
---------------------------------------------------------------------
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:46 2006