On Fri, 2009-01-09 at 06:09 -0800, Senthil Kumaran S wrote:
> Author: stylesen
> Date: Fri Jan 9 06:09:58 2009
> New Revision: 35112
>
> Log:
> Follow up r35055.
>
> * subversion/libsvn_subr/cmdline.c
> (svn_cmdline_create_auth_baton): For platform specific providers pass prompt
> function.
> * subversion/libsvn_subr/auth.c
> (svn_auth_get_platform_specific_provider): Change signature and include logic
> to accept prompt function and a boolean to indicate whether we are called
> from command line.
> (svn_auth_get_platform_specific_client_providers): Same as above.
> * subversion/include/svn_auth.h
> (svn_auth_get_platform_specific_provider): Reflect above changes.
> (svn_auth_get_platform_specific_client_providers): Reflect above changes.
> * subversion/tests/libsvn_subr/auth-test.c
> (test_platform_specific_auth_providers): Modify to accomodate above changes.
>
> Suggested by: rhuijben
> arfrever
> julianfoad
> hwright
>
> Modified:
> trunk/subversion/include/svn_auth.h
> trunk/subversion/libsvn_subr/auth.c
> trunk/subversion/libsvn_subr/cmdline.c
> trunk/subversion/tests/libsvn_subr/auth-test.c
>
> Modified: trunk/subversion/include/svn_auth.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_auth.h?pathrev=35112&r1=35111&r2=35112
> ==============================================================================
> --- trunk/subversion/include/svn_auth.h Fri Jan 9 05:19:51 2009 (r35111)
> +++ trunk/subversion/include/svn_auth.h Fri Jan 9 06:09:58 2009 (r35112)
> @@ -822,6 +822,13 @@ svn_auth_get_simple_provider(svn_auth_pr
> * Valid @a provider_type values are: "simple", "ssl_client_cert_pw" and
> * "ssl_server_trust".
> *
> + * @a *pb is the prompt provider baton for any prompts that should be created.
It would be helpful to say that this is the baton that will be passed to
@a prompt_func when it is called. If that's the case, then can we name
this argument 'prompt_baton' and put it immediately after 'prompt_func'
argument? I think that would help the reader understand it and would be
consistent with everywhere else.
> + * @a prompt_func is the actual prompt function for this provider.
For which provider?
I'm probably misunderstanding something here. Please help me to
understand.
I thought the idea of this function is that the caller doesn't know
which provider is needed, so it asks this function to work it out. This
function returns the answer: "here is the provider you need". So how
does the caller know in advance what prompt function is going to be
needed for that provider?
> + * @a command_line boolean value is set to TRUE if we are called from
> + * command line.
Please describe functions in terms of how they promise to behave, not in
terms of where they are called from. So, for this parameter, what does
it mean? Why does the function care and what does it do differently
depending on the value of this parameter?
Try to describe it without saying "command-line" at all. After all,
there is no hard and fast definition of what "command-line" means: for
example, a GUI could have a command-line mode built in, and then would
the argument's value be true or false? (That's just a rhetorical
question.)
If it is really helpful for the reader, you can add a note at the end of
a doc string about how the function is _likely_ to be called: for
example, "Typically, a command-line client might provide a prompt
function here whereas a GUI client might have obtained the answer
earlier and stored it in XXX and so will not need to provide a prompt
callback."
> + * Both @a *pb and @a prompt_func can be NULL if @a command_line is FALSE.
The answer to my questions above would probably explain what it means if
these arguments are NULL.
> + *
> * Allocate @a *provider in @a pool.
> *
> * What actually happens is we invoke the appropriate provider function to
> @@ -832,10 +839,14 @@ svn_auth_get_simple_provider(svn_auth_pr
> * @since New in 1.6.
> */
> svn_error_t *
> -svn_auth_get_platform_specific_provider(svn_auth_provider_object_t **provider,
> - const char *provider_name,
> - const char *provider_type,
> - apr_pool_t *pool);
> +svn_auth_get_platform_specific_provider
> + (svn_auth_provider_object_t **provider,
> + void *pb,
> + const char *provider_name,
> + const char *provider_type,
> + svn_auth_unlock_prompt_func_t prompt_func,
> + svn_boolean_t command_line,
> + apr_pool_t *pool);
>
Thanks.
- Julian
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1013960
Received on 2009-01-09 16:53:07 CET