From: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
Date: Fri, 12 Feb 2010 11:24:59 GMT
Subject: Re: [PATCH v2 2/2] git-imap-send: Convert LF to CRLF before storing patch to draft box
Message-ID: <4B753A8B.1090802@dcl.info.waseda.ac.jp>
In-Reply-To: <7viqa31otz.fsf@alter.siamese.dyndns.org>

On 2010年02月12日 05:48, Junio C Hamano wrote:
> Hitoshi Mitake<mitake@dcl.info.waseda.ac.jp>  writes:
>
>> According to RFC of IMAP, all messages must not have "bare newlines ('\n')".
>> '\n' should be converted to "\r\n" before storing messages to IMAP's mailbox.
>> This patch implements the converting function to git-imap-send.
>>
>> Cc: Erik Faye-Lund<kusmabite@googlemail.com>
>> Cc: Jakub Narebski<jnareb@gmail.com>
>> Cc: Linus Torvalds<torvalds@linux-foundation.org>
>> Cc: Jeff King<peff@peff.org>
>> Signed-off-by: Hitoshi Mitake<mitake@dcl.info.waseda.ac.jp>
>> ---
>>   imap-send.c |   25 +++++++++++++++++++++++++
>>   1 files changed, 25 insertions(+), 0 deletions(-)
>>
>> diff --git a/imap-send.c b/imap-send.c
>> index dcd8a2a..dbc72ca 100644
>> --- a/imap-send.c
>> +++ b/imap-send.c
>> @@ -1279,6 +1279,30 @@ static int imap_make_flags(int flags, char *buf)
>>   	return d;
>>   }
>>
>> +static void lf_to_crlf(struct msg_data *msg)
>> +{
>> +	char *new;
>> +	int i, j, lfnum = 0;
>> +
>> +	for (i = 0; i<  msg->len; i++) {
>> +		if (msg->data[i] == '\n')
>> +			lfnum++;
>> +	}
>> +	new = xcalloc(msg->len + lfnum, sizeof(char));
>> +	for (i = 0, j = 0; i<  msg->len; i++) {
>> +		if (msg->data[i] != '\n') {
>> +			new[j++] = msg->data[i];
>> +			continue;
>> +		}
>> +		new[j++] = '\r';
>> +		new[j++] = '\n';
>> +	}
>> +	msg->len += lfnum;
>> +	free(msg->data);
>> +	msg->data = new;
>> +	msg->crlf = 1;
>> +}
>
> Thanks.
>
> Two questions:
>
>   - "msg->crlf" -- what is it used for?  Do we need to maintain it?

This is old legacy from isync, and has no meaning now.
I removed it, thanks.

>
>   - Can the incoming payload already be CRLF terminated?  If so, do we want
>     to convert it into CRCRLF?
>

I didn't thought about the case.
I rewrote lf_to_crlf() for such case.

Thanks for your review.