[PATCH] open O_TEXT and O_BINARY for cygwin/windows

[Dave Dykstra]

On Fri, Jan 24, 2003 at 08:57:02AM +0200, Ville Herva wrote:
> On Thu, Jan 23, 2003 at 01:55:40PM -0600, you [Dave Dykstra] wrote:
> > Why did you skip the fopen in log.c, which appends to the log file?
> 
> I thought about that for a while. My reasoning was that the log file is
> probably read with unix/cygwin tools anyway - if not, the administrator is
> free to mount the filesystem binary or ascii. So I didn't want to enforce
> CR/LF on anyone.
> 
> In general, I think forcing ascii makes more sense for reading than writing,
> because it in most imaginable cases will do no harm.
>  
> > Because of the question about whether or not the "t" is ignored on all
> > platforms, I'm a little nervous about putting this into 2.5.6.  
> 
> It could be ifdefed, would it be safe to assume the following to work?
> 
> #if !defined(O_TEXT)
> #define O_TEXT 0
> #define O_TEXT_STR ""
> #else 
> #define O_TEXT_STR "t"
> #endif
> #if !defined(O_BINARY)
> #define O_BINARY 0
> #define O_BINARY_STR ""
> #else
> #define O_BINARY_STR "b"
> #endif
> 
> and then instead of
> 
>   fopen(foo, "rt");
> 
> do this
> 
>   fopen(foo, "r" O_TEXT_STR);
> 
> Comments?
> 
> > I don't have any problem with it for 2.6.0.  Maybe it should be just in
> > the 'patches' directory this time.
> > 
> > Did you check to see whether any of the code that's reading config files
> > is ignoring carriage returns at the end of lines anyway?
> 
> ISTR at least the password files were not recognized and I *think*
> rsync.conf was not parsed correctly when I originally wrote the patch.
> 
> Looking at the source, though
> 
> - exclude.c uses fgets() to read the lines from exclude file. I don't think 
>   it handles '\r'
> - param.c Ignores all '\r' in values, so rsync.conf should be fine. I'm not
>   sure, whether reading the file as text would be more elegant (in theory
>   there _could_ be other issues than the \r\n thing...)
> - authentivate.c seems to skip '\r' for passwords as well.
> 
> So I guess you are right for the most crucial part of code (as of current
> rsync).


I think I'll go ahead and put in your patch with the modification of using
O_TEXT_STR as you suggest.  I think the risk is low.  I had been concerned
that perhaps older CPPs might not be able to handle that syntax, but I see
other places in the rsync code that are depending on it so I think it's OK.

- Dave