Re: [PATCH v3 2/4] imap-send: add wrapper to get server credentials if needed

[Junio C Hamano <gitster@pobox.com>]

Nicolas Morey-Chaisemartin <nicolas@morey-chaisemartin.com> writes:

> Signed-off-by: Nicolas Morey-Chaisemartin <nicolas@morey-chaisemartin.com>
> ---
>  imap-send.c | 34 ++++++++++++++++++++--------------
>  1 file changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/imap-send.c b/imap-send.c
> index 09f29ea95..448a4a0b3 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -926,6 +926,25 @@ static int auth_cram_md5(struct imap_store *ctx, struct imap_cmd *cmd, const cha
>  	return 0;
>  }
>  
> +static void server_fill_credential(struct imap_server_conf *srvc, struct credential *cred)
> +{
> +	if (srvc->user && srvc->pass)
> +		return;
> +
> +	cred->protocol = xstrdup(srvc->use_ssl ? "imaps" : "imap");
> +	cred->host = xstrdup(srvc->host);
> +
> +	cred->username = xstrdup_or_null(srvc->user);
> +	cred->password = xstrdup_or_null(srvc->pass);
> +
> +	credential_fill(cred);
> +
> +	if (!srvc->user)
> +		srvc->user = xstrdup(cred->username);
> +	if (!srvc->pass)
> +		srvc->pass = xstrdup(cred->password);
> +}
> +

This looks straight-forward code movement.  The only thing that
makes me wonder is if this is "server".  The existing naming of the
variables screams at me that this is not "server" but is "service".

>  static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *folder)
>  {
>  	struct credential cred = CREDENTIAL_INIT;
> @@ -1078,20 +1097,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *f
>  		}
>  #endif
>  		imap_info("Logging in...\n");
> -		if (!srvc->user || !srvc->pass) {
> -			cred.protocol = xstrdup(srvc->use_ssl ? "imaps" : "imap");
> -			cred.host = xstrdup(srvc->host);
> -
> -			cred.username = xstrdup_or_null(srvc->user);
> -			cred.password = xstrdup_or_null(srvc->pass);
> -
> -			credential_fill(&cred);
> -
> -			if (!srvc->user)
> -				srvc->user = xstrdup(cred.username);
> -			if (!srvc->pass)
> -				srvc->pass = xstrdup(cred.password);
> -		}
> +		server_fill_credential(srvc, &cred);
>  
>  		if (srvc->auth_method) {
>  			struct imap_cmd_cb cb;