[patch] Always open files in binary mode

[Alp Toker]

On 24/02/2014 12:36, Rafael Espíndola wrote:
> On 24 February 2014 01:46, Alp Toker <alp at nuanti.com> wrote:
>> Hi Rafael,
>>
>> This makes a lot of sense. Supporting the Windows text mode flag only ever
>> caused bugs that were difficult to detect where F_Binary was mistakenly
>> omitted.
>>
>> Furthermore if we do ever want to write out CRLFs, we'll want to do it
>> explicitly by writing out '\r\n' instead of relying on the Windows feature
>> given that it introduces byte offsets that break seek/fseek.
>>
>> Your patch includes context for the cases where F_Binary was removed, but
>> what'd be more interesting is to see the raw_fd_ostream uses that _were_
>> previously operating in text mode whose functionality will change following
>> your patch. Do you have an idea which (if any) uses are still in text mode?
> Yes. Clang and llc had explicit logic for setting F_Binary only when
> not printing assembly. Things that knew they were always printing text
> (like a graphviz file), would also print in text mode. This is
> probably an historical thing. We  had text output only first and then
> added binary.
>
> If you want, I can make a first patch that just removes the default
> argument. That should make the F_None explicit where a text output is
> being used.

Yep, either that or making F_Binary the default and providing an F_Text 
seems reasonable before pulling the rug completely from under so we have 
an idea where the functional changes are, including other modules where 
Windows test coverage might not be great.

Alp.


>
> Cheers,
> Rafael

-- 
http://www.nuanti.com
the browser experts