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 -- To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html