Re: [PATCH] Fixed Cygwin CR-munging problem in mailsplit

From: Junio C Hamano <junkio@cox.net>
Date: May 27, 2006, 18:21 UTC
Message-ID: <7vfyivfhw4.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <44789309.1030002@Intel.com>

Salikh Zakirov <Salikh.Zakirov@Intel.com> writes:

Show 22 quoted lines

> Do not open mailbox file as fopen(..., "rt")
> as this strips CR characters from the diff,
> thus breaking the patch context for changes 
> in CRLF files.
>
> Signed-off-by: Salikh Zakirov <Salikh.Zakirov@Intel.com>
>
> ---
> fd405a0843f3efd474bc7897b06d813d6498fbf4
> diff --git mailsplit.c mailsplit.c
> index c529e2d..70a569c 100644
> --- mailsplit.c
> +++ mailsplit.c
> @@ -162,7 +162,7 @@ int main(int argc, const char **argv)
>  
>  	while (*argp) {
>  		const char *file = *argp++;
> -		FILE *f = !strcmp(file, "-") ? stdin : fopen(file, "rt");
> +		FILE *f = !strcmp(file, "-") ? stdin : fopen(file, "r");
>  		int file_done = 0;
>  
>  		if ( !f )

I personally think this is a right change. Provided if MTAs on the path between patch originator and you are not broken and your MUA saved the message with CR/LF distinction in the contents intact, this should do more right thing.

I see broken patches every once in a while, but when they are mangled by the mailpath, CRLF is the least of the problem; they have other whitespace breakage that makes them unapplicable anyway.

Having said that, however, that historically used to be a big IF with capital letters.

I have a feeling that Linus did this on purpose. For the projects we originally cared about, a patch to introduce CRLF in the tracked content was a broken patch 100% of the time (not 99%), and most likely caused by a breakage somewhere on the mailpath. At least in the original git context, protecting UNIX/POSIX people from broken MTA/MUA counted far more than catering to people who deals with DOSsy contents.

So I am slightly in favor of the change, but just barely.