Stefan Sperling wrote on Tue, 15 Jul 2008 at 18:51 +0200:
> On Tue, Jul 15, 2008 at 11:39:12AM -0400, Karl Fogel wrote:
> > stsp_at_tigris.org writes:
> > > Log:
> > > * subversion/libsvn_subr/simple_providers.c:
> > > (svn_auth__simple_save_creds_helper): Test a pointer pointing
> > > to a boolean against NULL explicitly. The code as written made
> > > it relatively easy to confuse the pointer with the boolean itself.
> > > No functional change.
> >
> > +1 on this change, but just a heads up, stsp: I'm planning to rewrite
> > that code to use a plain boolean (instead of a pointer to boolean), for
> > simplicity. You can see how it will look by examining:
> >
> > http://subversion.tigris.org/nonav/issues/showattachment.cgi/907/2489-patch-v5.txt
> >
> > which has similar code (e.g., same variable names) in another file.
> >
> > (I don't want to change simple_providers.c until I've committed the
> > above patch, which is undergoing 'make check' right now.)
>
> You mean this bit, right?
>
> + /* We're interactive, and the client provided a
> + * prompt callback. So we can ask the user.
> + *
> + * Check for a cached answer before prompting. */
> + svn_boolean_t cached_answer =
> + (svn_boolean_t) apr_hash_get(b->plaintext_answers,
> + realmstring,
> + APR_HASH_KEY_STRING);
I might be overlooking something (again), but how does this code
distinguish between "no answer is cached" (apr_hash_get() returns NULL)
and "an answer is cached, and it is 'FALSE'" (apr_hash_get() returns
FALSE)?
Also, why does this use APR_HASH_KEY_STRING? A pointer -- or a boolean
-- aren't a string.
> + if (cached_answer)
> + may_save_passphrase = cached_answer;
> + else
> + {
> + /* Nothing cached for this realm, prompt the user. */
> + SVN_ERR((*b->plaintext_passphrase_prompt_func)
> + (&may_save_passphrase,
> + realmstring,
> + b->prompt_baton,
> + pool));
The corresponding apr_hash_set is:
> + cached_answer = may_save_passphrase;
> + apr_hash_set(b->plaintext_answers, realmstring,
> + APR_HASH_KEY_STRING,
> + (void *) cached_answer);
---------------------------------------------------------------------
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-15 19:49:12 CEST