Re: svn commit: r37322 - trunk/subversion/libsvn_auth_gnome_keyring

Bert Huijben wrote:
>
>> -----Original Message-----
>> From: Senthil Kumaran S [mailto:senthil_at_collab.net]
>> Sent: vrijdag 17 april 2009 15:43
>> To: svn_at_subversion.tigris.org
>> Subject: svn commit: r37322 - trunk/subversion/libsvn_auth_gnome_keyring
>>
>> Author: stylesen
>> Date: Fri Apr 17 06:42:45 2009
>> New Revision: 37322
>>
>> Log:
>> Prevent opening GNOME dialog in order to get the password for default
> keyring,
>> alternatively get it through a prompt.
>>
>> * subversion/libsvn_auth_gnome_keyring/gnome_keyring.c
>> (unlock_gnome_keyring): Make this function to return a boolean value on
>> unlocking keyring.
>> (simple_gnome_keyring_first_creds, simple_gnome_keyring_save_creds,
>> ssl_client_cert_pw_gnome_keyring_first_creds,
>> ssl_client_cert_pw_gnome_keyring_save_creds): Try unlocking a keyring,
> if
>> not move on to the next provider.
>>
>> Modified:
>> trunk/subversion/libsvn_auth_gnome_keyring/gnome_keyring.c
>>
>> Modified: trunk/subversion/libsvn_auth_gnome_keyring/gnome_keyring.c
>> URL:
>>
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_auth_gnome_keyring/
> gn
>> ome_keyring.c?pathrev=37322&r1=37321&r2=37322
>>
> ============================================================================
> ==
>> --- trunk/subversion/libsvn_auth_gnome_keyring/gnome_keyring.c Fri
> Apr 17
>> 05:39:58 2009 (r37321)
>> +++ trunk/subversion/libsvn_auth_gnome_keyring/gnome_keyring.c Fri
> Apr 17
>> 06:42:45 2009 (r37322)
>> @@ -199,8 +199,9 @@ check_keyring_is_locked(const char *keyr
>> return FALSE;
>> }
>>
>> -/* Unlock the KEYRING_NAME with the KEYRING_PASSWORD. */
>> -static void
>> +/* Unlock the KEYRING_NAME with the KEYRING_PASSWORD. If KEYRING was
>> + successfully unlocked return TRUE. */
>> +static svn_boolean_t
>> unlock_gnome_keyring(const char *keyring_name,
>> const char *keyring_password,
>> apr_pool_t *pool)
>> @@ -220,7 +221,7 @@ unlock_gnome_keyring(const char *keyring
>> if (key_info.info == NULL)
>> {
>> callback_destroy_data_keyring((void*)&key_info);
>> - return;
>> + return FALSE;
>> }
>> else
>> {
>> @@ -231,7 +232,10 @@ unlock_gnome_keyring(const char *keyring
>> g_main_loop_run(key_info.loop);
>> }
>> callback_destroy_data_keyring((void*)&key_info);
>> - return;
>> + if (check_keyring_is_locked(keyring_name))
>> + return FALSE;
>> +
>> + return TRUE;
>> }
>>
>> /* Implementation of password_get_t that retrieves the password
>> @@ -385,6 +389,7 @@ simple_gnome_keyring_first_creds(void **
>> APR_HASH_KEY_STRING);
>>
>> char *keyring_password;
>> + svn_boolean_t unlocked = FALSE;
>> const char *default_keyring = get_default_keyring_name(pool);
>>
>> if (check_keyring_is_locked(default_keyring))
>> @@ -395,8 +400,12 @@ simple_gnome_keyring_first_creds(void **
>> default_keyring,
>> unlock_prompt_baton,
>> pool));
>> - unlock_gnome_keyring(default_keyring, keyring_password,
>> - pool);
>> + unlocked = unlock_gnome_keyring(default_keyring,
>> + keyring_password, pool);
>> +
>> + /* If keyring is locked give up and try the next provider.
> */
>> + if (! unlocked)
>> + return SVN_NO_ERROR;
>> }
>> }
>> }
>
>
> Why do you define unlocked at the top level of the function. You could
> reduce the scope (or just check the result without storing it in a
> variable).

Done in r37325.

Thank You.

-- 
Senthil Kumaran S
http://www.stylesen.org/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1768602