Re: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
Dave Dykstra wrote: I suspect it's also the reason why the build.samba.org cygwin machine hasn't reported a result in the last 9 hours. Nope.. problems with the CPU fan-cooler =( I'm taking it back out and washing my hands of the cygwin rsync port, I'm fed up. I'll catch up with Max doscovering and do further testing after tomorrow's test. -- Lapo 'Raist' Luchini [EMAIL PROTECTED] (PGP X.509 keys available) http://www.lapo.it (ICQ UIN: 529796) -- To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html
Re: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
On Tue, Jan 28, 2003 at 01:26:32PM -, Max Bowsher wrote: Dave Dykstra wrote: I give up. The msleep(100) consistently causes hangs of the unsafe-links test on my friend's cygwin Windows 2000 machine. I suspect it's also the reason why the build.samba.org cygwin machine hasn't reported a result in the last 9 hours. I'm taking it back out and washing my hands of the cygwin rsync port, I'm fed up. You've been most considerate in actively seeking out and merging Cygwin patches for this release. Alas, it backfired in merging the msleep thing. I definitely agree with it not being in the upstream sources. Making a race condition harder to hit, with a simple delay, is obviously not a fix. You may be pleased to know that the SHUTDOWN_ALL_SOCKETS change has eliminated Connection reset by peer messages when running the testsuite. Well it's good to know that one of the cygwin changes did some good. It may be that the hang has just vanished. If it hasn't, it's probably a bug in Cygwin, not rsync. Hopefully we'll get a nice bugreport, and it can be traced and squashed. Hopefully. - 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
Re: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
Dave Dykstra wrote: What, in particular? I'm not a very good testcase, because I use binary mounts and unix line endings everywhere. It compiles and does syncs with remote rsync daemons, which is my normal usage. Max. See if exclude files with DOS line endings work ok for you, and also the other config files if you can. - Dave I can test that before releasing cygwin's 2.5.6-1 package, unfortunately I have almost *no* free time until after friday (university test on thursday ^_^) Anyway Max is also right, local issues can be resolved locally with much speed, even if in the long run it's of course better to put them in rsync cvs too. BTW: I wonder if the sleepms(30) is still needed, it's quite a bit of time that my build.samba.org hourly cron doesn't produce a hanged rsync... (must check that too) -- Lapo 'Raist' Luchini [EMAIL PROTECTED] (PGP X.509 keys available) http://www.lapo.it (ICQ UIN: 529796) -- To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html
Re: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
Dave Dykstra wrote: On Mon, Jan 27, 2003 at 10:46:16AM +0100, Lapo Luchini wrote: I can test that before releasing cygwin's 2.5.6-1 package, unfortunately I have almost *no* free time until after friday (university test on thursday ^_^) Anyway Max is also right, local issues can be resolved locally with much speed, even if in the long run it's of course better to put them in rsync cvs too. I'm reluctant to rely on cygwin-stream-specific patches because I know that other people compile it from source too. Probably the msleep should be put in for Cygwin only with an ifdef though, or the waiting of the child removed altogether on Cygwin only. Does it not make sense that anyone wishing to compile their own rsync for Cygwin should start by looking at the Cygwin package? (Which, btw, includes the clean rsync source, and patchfiles, so it is easy to see what is being done to rsync specially for Cygwin). The waiting for the child is presumably there for a reason. If is removed for Cygwin, it should probably be removed for all platforms. If there is an argument against that, then it applies equally to Cygwin as to any other platforms. BTW: I wonder if the sleepms(30) is still needed, it's quite a bit of time that my build.samba.org hourly cron doesn't produce a hanged rsync... (must check that too) I know that some people still experience it, and I don't think anything has changed in that area since the time that the person who experienced the hang discovered the hack. Cygwin has gone through many minor versions since then. I suggest releasing an rsync without the hack, but with a command line option to turn it on. That way, we get to see for certain whether the problem still exists, and there is an easy workaround available if it does. Max. -- To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html
Re: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
On Mon, Jan 27, 2003 at 04:45:25PM -, Max Bowsher wrote: Dave Dykstra wrote: On Mon, Jan 27, 2003 at 10:46:16AM +0100, Lapo Luchini wrote: I can test that before releasing cygwin's 2.5.6-1 package, unfortunately I have almost *no* free time until after friday (university test on thursday ^_^) Anyway Max is also right, local issues can be resolved locally with much speed, even if in the long run it's of course better to put them in rsync cvs too. I'm reluctant to rely on cygwin-stream-specific patches because I know that other people compile it from source too. Probably the msleep should be put in for Cygwin only with an ifdef though, or the waiting of the child removed altogether on Cygwin only. Does it not make sense that anyone wishing to compile their own rsync for Cygwin should start by looking at the Cygwin package? (Which, btw, includes the clean rsync source, and patchfiles, so it is easy to see what is being done to rsync specially for Cygwin). In my case I used to use the same source code to compile on many different platforms and no, I wouldn't look at the Cygwin package first to compile on Cygwin. The waiting for the child is presumably there for a reason. If is removed for Cygwin, it should probably be removed for all platforms. If there is an argument against that, then it applies equally to Cygwin as to any other platforms. Perhaps the waiting in the child was put in for specific problems on some platforms. BTW: I wonder if the sleepms(30) is still needed, it's quite a bit of time that my build.samba.org hourly cron doesn't produce a hanged rsync... (must check that too) I know that some people still experience it, and I don't think anything has changed in that area since the time that the person who experienced the hang discovered the hack. Cygwin has gone through many minor versions since then. I suggest releasing an rsync without the hack, but with a command line option to turn it on. That way, we get to see for certain whether the problem still exists, and there is an easy workaround available if it does. So many people have complained about Cygwin hangs that I'm inclined to leave the msleep(100) in on this release by default, but only on Cygwin. When the problem is shown to be fixed on Cygwin, it can be taken back out. - 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
Re: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
On Mon, Jan 27, 2003 at 04:01:46PM -0600, Dave Dykstra wrote: On Mon, Jan 27, 2003 at 04:45:25PM -, Max Bowsher wrote: ... Cygwin has gone through many minor versions since then. I suggest releasing an rsync without the hack, but with a command line option to turn it on. That way, we get to see for certain whether the problem still exists, and there is an easy workaround available if it does. So many people have complained about Cygwin hangs that I'm inclined to leave the msleep(100) in on this release by default, but only on Cygwin. When the problem is shown to be fixed on Cygwin, it can be taken back out. I give up. The msleep(100) consistently causes hangs of the unsafe-links test on my friend's cygwin Windows 2000 machine. I suspect it's also the reason why the build.samba.org cygwin machine hasn't reported a result in the last 9 hours. I'm taking it back out and washing my hands of the cygwin rsync port, I'm fed up. - 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
RE: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
Ville Herva [mailto:[EMAIL PROTECTED]] wrote: Of course, whether O_TEXT is defined or not does not necessarily imply the availability of t, but I can't think of better alternative. Stratus VOS implements O_TEXT and O_BINARY but does not recognize t. We have the options defined in ANS C and in POSIX. I'm at home and don't have my reference materials handy, but I think this is going to be a problem for us. I can see looking at the code that we will reject the open if we see unknown letters. Please write a configure test. Even if we are wrong and we should be ignoring it, I have hundreds of machines in the field and it will be years before they are upgraded. The string concatenation thing is not an issue here. Thanks PG -- Paul Green, Senior Technical Consultant, Stratus Computer, Inc. Voice: +1 978-461-7557; FAX: +1 978-461-3610; Video on request. Speaking from Stratus not for Stratus -- To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html
Re: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
On Sun, Jan 26, 2003 at 09:43:06AM -0500, Green, Paul wrote:
Ville Herva [mailto:[EMAIL PROTECTED]] wrote:
Of course, whether O_TEXT is defined or not does not
necessarily imply the availability of t, but I
can't think of better alternative.
Stratus VOS implements O_TEXT and O_BINARY but does not recognize t. We
have the options defined in ANS C and in POSIX. I'm at home and don't have
my reference materials handy, but I think this is going to be a problem for
us. I can see looking at the code that we will reject the open if we see
unknown letters.
Please write a configure test. Even if we are wrong and we should be
ignoring it, I have hundreds of machines in the field and it will be years
before they are upgraded.
The string concatenation thing is not an issue here.
On the systems where O_TEXT and rt have meaning are we
looking at anything other than converting \r\n to \n?
If not i don't think we need the O_TEXT stuff.
I did a quick survey of where you have the O_TEXT and O_TEXT_STR
and of the one other calls to fopen (logfile).
authenticate.c: fd = open(fname,O_RDONLY | O_TEXT);
get_secret() already discards \r
authenticate.c: if ( (fd=open(filename,O_RDONLY | O_TEXT)) == -1) {
getpassf() treats \r the same as \n
clientserver.c: FILE *f = fopen(motd,r O_TEXT_STR);
simply passes contents of motd file unchanged to client
does it matter if there are \r chars in the file?
exclude.c: f = fopen(fname,r O_TEXT_STR);
exclude.c: f = fdopen(0, r O_TEXT_STR);
the file is processed with
while (fgets(line,MAXPATHLEN,f)) {
int l = strlen(line);
if (l line[l-1] == '\n') l--;
line[l] = 0;
if we add
if (l line[l-1] == '\r') l--;
after the \n processing that will handle the \r\n case
or we can replace the \n line with
while (l (line[l-1] == '\n'|| line[l-1] == '\r')) l--;
and this function will be covered.
params.c: OpenedFile = fopen( FileName, r O_TEXT_STR );
since \r is a whitespace character it should be
ignored.
Based on this i don't think we need to worry about O_TEXT or
rt mode. The only place i can imagine we might want text
mode is the log file and i imagine that admins can cope with
EOL issues.
--
J.W. SchultzPegasystems Technologies
email address: [EMAIL PROTECTED]
Remember Cernan and Schmitt
--
To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html
Re: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
On Sun, Jan 26, 2003 at 08:16:50AM -0800, jw schultz wrote:
authenticate.c: fd = open(fname,O_RDONLY | O_TEXT);
get_secret() already discards \r
authenticate.c: if ( (fd=open(filename,O_RDONLY | O_TEXT)) == -1) {
getpassf() treats \r the same as \n
Yeah, these already handle both Mac and PC line endings, so I removed
the O_TEXT bit from these.
clientserver.c: FILE *f = fopen(motd,r O_TEXT_STR);
simply passes contents of motd file unchanged to client
does it matter if there are \r chars in the file?
I'm trying to imagine what will happen for Mac systems as well. Since
the default for files is to open them in text mode on a Mac (and they
don't have the Cygwin weirdness of trying to override a binary-mode
mount, right?), then I think you're right here -- the extra \r that
might come from a PC system should hopefully cause no problems (but we
should test this). In the meantime, I think it would be safe to revert
this change.
exclude.c: f = fopen(fname,r O_TEXT_STR);
exclude.c: f = fdopen(0, r O_TEXT_STR);
the file is processed with
while (fgets(line,MAXPATHLEN,f)) {
int l = strlen(line);
if (l line[l-1] == '\n') l--;
line[l] = 0;
if we add
if (l line[l-1] == '\r') l--;
after the \n processing that will handle the \r\n case
or we can replace the \n line with
while (l (line[l-1] == '\n'|| line[l-1] == '\r')) l--;
and this function will be covered.
As long as Mac systems are reading the file in text mode without the
O_TEXT_STR, we should be fine (and I believe that to be the case).
params.c: OpenedFile = fopen( FileName, r O_TEXT_STR );
since \r is a whitespace character it should be
ignored.
Sounds right (though I haven't verified your analysis).
Since it sounds like the t has a potential for problems, I'm taking
these recent changes out and will just put in the one suggested change
to ignore a '\r' at the end of the line. This should be safer for the
current release.
..wayne..
--
To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html
Re: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
Dave Dykstra wrote:
Alright. Max, could you quickly verify if the latest CVS version
works OK for you on Cygwin?
What, in particular? I'm not a very good testcase, because I use binary
mounts and unix line endings everywhere.
It compiles and does syncs with remote rsync daemons, which is my normal
usage.
Max.
It's better to always handle all three styles of line terminations
anyway, because there are other situations than systems that have
O_TEXT defined where it might be wanted, for example a Linux system
that has a Windows filesystem mounted.
- Dave
On Sun, Jan 26, 2003 at 12:07:07PM -0800, Wayne Davison wrote:
On Sun, Jan 26, 2003 at 08:16:50AM -0800, jw schultz wrote:
authenticate.c: fd = open(fname,O_RDONLY | O_TEXT);
get_secret() already discards \r
authenticate.c: if ( (fd=open(filename,O_RDONLY | O_TEXT)) == -1) {
getpassf() treats \r the same as \n
Yeah, these already handle both Mac and PC line endings, so I removed
the O_TEXT bit from these.
clientserver.c: FILE *f = fopen(motd,r O_TEXT_STR);
simply passes contents of motd file unchanged to client
does it matter if there are \r chars in the file?
I'm trying to imagine what will happen for Mac systems as well.
Since the default for files is to open them in text mode on a Mac
(and they don't have the Cygwin weirdness of trying to override a
binary-mode mount, right?), then I think you're right here -- the
extra \r that might come from a PC system should hopefully cause no
problems (but we should test this). In the meantime, I think it
would be safe to revert this change.
exclude.c: f = fopen(fname,r O_TEXT_STR);
exclude.c: f = fdopen(0, r O_TEXT_STR);
the file is processed with
while (fgets(line,MAXPATHLEN,f)) {
int l = strlen(line);
if (l line[l-1] == '\n') l--;
line[l] = 0;
if we add
if (l line[l-1] == '\r') l--;
after the \n processing that will handle the \r\n case
or we can replace the \n line with
while (l (line[l-1] == '\n'|| line[l-1] == '\r')) l--;
and this function will be covered.
As long as Mac systems are reading the file in text mode without the
O_TEXT_STR, we should be fine (and I believe that to be the case).
params.c: OpenedFile = fopen( FileName, r O_TEXT_STR );
since \r is a whitespace character it should be
ignored.
Sounds right (though I haven't verified your analysis).
Since it sounds like the t has a potential for problems, I'm taking
these recent changes out and will just put in the one suggested
change to ignore a '\r' at the end of the line. This should be
safer for the current release.
...wayne..
--
To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html
Re: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
On Sun, Jan 26, 2003 at 08:54:19PM -, Max Bowsher wrote: Dave Dykstra wrote: Alright. Max, could you quickly verify if the latest CVS version works OK for you on Cygwin? What, in particular? I'm not a very good testcase, because I use binary mounts and unix line endings everywhere. It compiles and does syncs with remote rsync daemons, which is my normal usage. Max. See if exclude files with DOS line endings work ok for you, and also the other config files if you can. - 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
Re: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
Dave Dykstra wrote: On Sun, Jan 26, 2003 at 08:54:19PM -, Max Bowsher wrote: Dave Dykstra wrote: Alright. Max, could you quickly verify if the latest CVS version works OK for you on Cygwin? What, in particular? I'm not a very good testcase, because I use binary mounts and unix line endings everywhere. It compiles and does syncs with remote rsync daemons, which is my normal usage. Max. See if exclude files with DOS line endings work ok for you, and also the other config files if you can. Sorry, I don't have time to fiddle around to that extent right now. If you need to get 2.5.6 out the door, then do. If the Cygwin 2.5.6-1 doesn't work quite right, there can always be a 2.5.6-2. Max. -- To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html
Re: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
On Fri, Jan 24, 2003 at 05:09:43PM -0600, you [Dave Dykstra] wrote: I think I'll go ahead and put in your patch with the modification of using O_TEXT_STR as you suggest. Thanks. I think the risk is low. I think so, too. 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. I think it is standard C, but OTOH looking at for example http://www.chris-lott.org/resources/cstyle/portableC.pdf reminds that not all ancient C preprocessors are compliant. But, with such ancient beast, I think a lot of other things would break, too. -- v -- [EMAIL PROTECTED] -- To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html
Re: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
On Sat, Jan 25, 2003 at 10:56:37AM +0200, Ville Herva wrote: On Fri, Jan 24, 2003 at 05:09:43PM -0600, you [Dave Dykstra] wrote: I think I'll go ahead and put in your patch with the modification of using O_TEXT_STR as you suggest. Thanks. I think the risk is low. I think so, too. 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. I think it is standard C, but OTOH looking at for example http://www.chris-lott.org/resources/cstyle/portableC.pdf reminds that not all ancient C preprocessors are compliant. But, with such ancient beast, I think a lot of other things would break, too. fopen() spec per SUSv3 (IEEE Std 1003.1-2001) which supercedes POSIX: The mode argument points to a string. If the string is one of the following, the file shall be opened in the indicated mode. Otherwise, the behavior is undefined. r or rb Open file for reading. w or wb Truncate to zero length or create file for writing. a or ab Append; open or create file for writing at end-of-file. r+ or rb+ or r+b Open file for update (reading and writing). w+ or wb+ or w+b Truncate to zero length or create file for update. a+ or ab+ or a+b Append; open or create file for update, writing at end-of-file. The character 'b' shall have no effect, but is allowed for ISO C standard conformance. Opening a file with read mode (r as the first character in the mode argument) shall fail if the file does not exist or cannot be read. Annotations therewith indicate that this is aligned with the ISO C standard and any conflict is unintentional and defers to the ISO C standard. What i don't see here is any mention of the t option. b is explicitly permissable which would imply t is otherwise assumed. Curiously, the spec does not permit fopen to fail if the mode argument contains unrecognized characters. The only failures permitted related to mode have to do with access privileges. SUSv3 does not show the availability of a O_BINARY or O_TEXT mode for open() which makes sense. Text/binary handling is not the responsibility of the OS and open() is a system call. I think you will need the macros for the open() calls. -- J.W. SchultzPegasystems Technologies email address: [EMAIL PROTECTED] Remember Cernan and Schmitt -- To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html
Re: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
On Sat, Jan 25, 2003 at 01:43:39AM -0800, you [jw schultz] wrote: On Sat, Jan 25, 2003 at 10:56:37AM +0200, Ville Herva wrote: On Fri, Jan 24, 2003 at 05:09:43PM -0600, you [Dave Dykstra] wrote: I think I'll go ahead and put in your patch with the modification of using O_TEXT_STR as you suggest. Thanks. I think the risk is low. I think so, too. 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. I think it is standard C, but OTOH looking at for example http://www.chris-lott.org/resources/cstyle/portableC.pdf reminds that not all ancient C preprocessors are compliant. But, with such ancient beast, I think a lot of other things would break, too. fopen() spec per SUSv3 (IEEE Std 1003.1-2001) which supercedes POSIX: (...) Annotations therewith indicate that this is aligned with the ISO C standard and any conflict is unintentional and defers to the ISO C standard. To clear any confusion: With I think it is standard C I wasn't referring to use of rt per se but to the macro trick I suggested earlier in this thread (and to which Dave agreed to). Namely: #ifdef O_TEXT #define O_TEXT_STR t #else #define O_TEXT_STR #endif and then fopen(filename, r O_TEXT_STR); Dave brought up his concerns about the use of C preprocessor string catenation, but noted that the same trcik is already used elsewhere in rsync source. Of course, whether O_TEXT is defined or not does not necessarily imply the availability of t, but I can't think of better alternative. What i don't see here is any mention of the t option. b is explicitly permissable which would imply t is otherwise assumed. But it isn't on Cygwin, at least when the mount is of binary type. Curiously, the spec does not permit fopen to fail if the mode argument contains unrecognized characters. The only failures permitted related to mode have to do with access privileges. Well, that is good. SUSv3 does not show the availability of a O_BINARY or O_TEXT mode for open() which makes sense. Text/binary handling is not the responsibility of the OS and open() is a system call. I think you will need the macros for the open() calls. -- v -- [EMAIL PROTECTED] -- To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html
Re: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
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. Yes, please don't write in explicit text mode. Keeping the flexibility here is good. Max. -- To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html
Re: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
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
Re: [PATCH] open O_TEXT and O_BINARY for cygwin/windows
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). -- v -- [EMAIL PROTECTED] -- To unsubscribe or change options: http://lists.samba.org/mailman/listinfo/rsync Before posting, read: http://www.tuxedo.org/~esr/faqs/smart-questions.html
[PATCH] open O_TEXT and O_BINARY for cygwin/windows
perhaps you would want to consider the force binary open for data files
on CYGWIN -patch I sent ages ago? I feel it's quite important, and to
me it never makes sense to open data files in ascii (CR/LF translation
mode) nor config files in BINARY mode (opening them in ascii makes it
possible to edit them with both NOTEPAD etc and the unix line ending
compliant cygwin editors.)
See the thread starting at:
http://groups.google.com/groups?hl=enlr=ie=UTF-8oe=utf-8threadm=a4vfnh%242ec%241%40FreeBSD.csie.NCTU.edu.twrnum=1prev=/groups%3Fq%3Drsync%2Bon%2Bcygwin%2B-%2Btextmode%2Bconfig%2Bfiles%26num%3D50%26hl%3Den%26lr%3D%26ie%3DUTF-8%26oe%3Dutf-8%26sa%3DN%26tab%3Dwg
I'm mostly talking about (4) - others are not that important.
On Wed, Jan 22, 2003 at 03:53:27PM -0600, you [Dave Dykstra] wrote:
Could you please port the patch to 2.5.6pre2 and post it to the mailing
list?
Here it is.
In short: on Cygwin you can mount volumes as text or binary. For text mounts
CR/LF-CR translation is done on the fly, binary mounts do not alter the
data. For applications that deal with raw data (gzip, tar, md5sum, cp,
rsync) it makes sense to access the data in binary always (and actually
rsync does nowadays, the O_BINARY flag is set in do_open). On the other hand
it makes sense to open config files in text so that the user can change
them with windows editors - the CR/LF translation will do no harm even if
the file has unix line endings.
This batch adds a few O_TEXT and O_BINARY flags to *open() calls - files
that are obviously text are opened as such; data files are opened as
binary.
- Files batch.c checksum.c generator.c receiver.c sender.c util.c use
do_open which does O_BINARY already (which has chanced since I originally
made the patch years ago), so no need to touch those. Other than that
I went through all open, fdopen, and fopen calls and tried to
add either O_TEXT or O_BINARY where it made sense.
- O_BINARY and O_TEXT are defined in rsync.h unless they are available
on the platform:
+#if !defined(O_TEXT)
+#define O_TEXT 0
+#endif
+#if !defined(O_BINARY)
+#define O_BINARY 0
+#endif
- I also use fopen(filename, rt). I _think_ that all fopen implementations
should ignore t unless they recognize it. At least linux does (it
seems to ignore all chars it doesn't recognize. rt is one of the
alternatives Chris Faylor recommends for CYGWIN:
http://www.cygwin.com/ml/cygwin/2000-10/msg00213.html:
You do that one of three ways:
open (foo, O_RDONLY | O_TEXT);
fopen (foo, rt);
setmode (fd, O_TEXT);
rt one is the only one for FILEs (and setmode doesn't even exists on
all other platforms AFAICT.
man fopen says:
The mode string can also include the letter ``b'' either
as a last character or as a character between the charac
ters in any of the two-character strings described above.
This is strictly for compatibility with ANSI X3.159-1989
(``ANSI C'') and has no effect; the ``b'' is ignored on
all POSIX conforming systems, including Linux. (Other
systems may treat text files and binary files differently,
and adding the ``b'' may be a good idea if you do I/O to a
binary file and expect that your program may be ported to
non-Unix environments.)
So I would hope that t is handled the same way.
Anyway, if someone knows better, please tell.
Patch attached - compiled, briefly tested (the previous incarnations tested
pretty extensively.) It doesn't alter the behaviour on platforms that don't
do O_BINARY and O_TEXT differently so I thinks it should be quite safe to
apply.
-- v --
[EMAIL PROTECTED]
diff -Naur --show-c-function --exclude='*.o' --exclude='*.exe' --exclude=Makefile
--exclude='*~' rsync-2.5.6pre2.ORIG/authenticate.c rsync-2.5.6pre2/authenticate.c
--- rsync-2.5.6pre2.ORIG/authenticate.c 2002-08-01 03:40:13.0 +0300
+++ rsync-2.5.6pre2/authenticate.c 2003-01-23 08:56:09.0 +0200
@@ -82,7 +82,7 @@ static int get_secret(int module, char *
if (!fname || !*fname) return 0;
- fd = open(fname,O_RDONLY);
+ fd = open(fname,O_RDONLY | O_TEXT);
if (fd == -1) return 0;
if (do_stat(fname, st) == -1) {
@@ -144,7 +144,7 @@ static char *getpassf(char *filename)
if (!filename) return NULL;
- if ( (fd=open(filename,O_RDONLY)) == -1) {
+ if ( (fd=open(filename,O_RDONLY | O_TEXT)) == -1) {
rsyserr(FERROR, errno, could not open password file \%s\,filename);
if (envpw) rprintf(FERROR,falling back to RSYNC_PASSWORD environment
variable.\n);
return NULL;
diff -Naur --show-c-function --exclude='*.o' --exclude='*.exe' --exclude=Makefile
--exclude='*~' rsync-2.5.6pre2.ORIG/clientserver.c rsync-2.5.6pre2/clientserver.c
--- rsync-2.5.6pre2.ORIG/clientserver.c 2002-08-31 02:30:08.0 +0300
+++ rsync-2.5.6pre2/clientserver.c
