From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 24 Aug 2017 18:54:42 GMT
Subject: Re: [PATCH v3 2/4] imap-send: add wrapper to get server credentials if needed
Message-ID: <xmqqh8wwx0ct.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <d1c39912-a847-1831-60ae-18b15359092f@morey-chaisemartin.com>

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

> Le 23/08/2017 à 22:13, Junio C Hamano a écrit :
>> 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".
>
> I read srvc as server conf not service.

Oh, yeah, "server conf" is OK (I didn't think of it).

> But I can change the name if you prefer

Nah.  Please keep it.