Senthil Kumaran S <senthil_at_collab.net> writes:
> Karl Fogel wrote:
>> A couple of places in libsvn_subr/cmdline.c have code like this:
>>
>> if (non_interactive == FALSE)
>> {
>> /* This provider doesn't prompt the user in order to get creds;
>> it prompts the user regarding the caching of creds. */
>> svn_auth_get_simple_provider2(&provider,
>> svn_cmdline_auth_plaintext_prompt,
>> pb, pool);
>> }
>> else
>> {
>> svn_auth_get_simple_provider2(&provider, NULL, NULL, pool);
>> }
>> APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *) = provider;
>> svn_auth_get_username_provider(&provider, pool);
>> APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *) = provider;
>>
>> But why do we bother to register a null provider in the non_interactive
>> case? Why not just avoid pushing onto the 'providers' array at all, in
>> that case?
>
> This is because we use the same "svn_auth_provider_object_t *provider"
> variable. For example, let us assume we have compiled with 'kwallet' where the
> following code is executed:
>
> <snip>
> #ifdef SVN_HAVE_KWALLET
> SVN_ERR(get_auth_simple_provider(&provider, "kwallet", pool));
> if (provider)
> {
> APR_ARRAY_PUSH(providers,
> svn_auth_provider_object_t *) = provider;
> }
> #endif
> continue;
> </snip>
>
> Once the above code is executed, we do not invalidate 'provider' variable,
> after which we call 'svn_auth_get_simple_provider2' and push the provider into
> the array. If there is no 'if...else' condition, then we may push the wrong
> provider to the array which is not correct. Hence it is good we
> register a null provider instead.
I'm saying, why don't we write the code like this instead:
if (non_interactive == FALSE)
{
/* This provider doesn't prompt the user in order to get creds;
it prompts the user regarding the caching of creds. */
svn_auth_get_simple_provider2(&provider,
svn_cmdline_auth_plaintext_prompt,
pb, pool);
APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *) = provider;
svn_auth_get_username_provider(&provider, pool);
APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *) = provider;
}
Then we don't even need an 'else' clause there, or in the other place
where the code is similarly arranged. There is no fall-through issue.
Am I missing anything?
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-22 01:48:46 CEST