Re: Weird loop in ftp client
Maxime Villard m...@m00nbsd.net writes:
Is there something wrong with this patch?
Nope, there is something wrong with ftp(1) and how it recovers from the
error. I'll commit this tomorrow unless someone disagrees.
Le 29/11/2013 20:44, Maxime Villard a écrit :
What about that?
Index: ftp.c
===
RCS file: /cvs/src/usr.bin/ftp/ftp.c,v
retrieving revision 1.83
diff -u -r1.83 ftp.c
--- ftp.c13 Nov 2013 20:41:14 - 1.83
+++ ftp.c29 Nov 2013 21:17:49 -
@@ -1089,9 +1089,10 @@
may_send_noop_char();
d = 0;
do {
-wr = write(fileno(fout), buf + d, rd);
-if (wr == -1 errno == EPIPE)
+if ((wr = write(fileno(fout), buf + d, rd)) ==
-1) {
+d = -1;
break;
+}
d += wr;
rd -= wr;
} while (d c);
p is set to -1 to display the proper error message
of course here I meant 'd'
--
jca | PGP: 0x06A11494 / 61DB D9A0 00A4 67CF 2A90 8961 6191 8FBF 06A1 1494
Re: Weird loop in ftp client
Is there something wrong with this patch?
Le 29/11/2013 20:44, Maxime Villard a écrit :
What about that?
Index: ftp.c
===
RCS file: /cvs/src/usr.bin/ftp/ftp.c,v
retrieving revision 1.83
diff -u -r1.83 ftp.c
--- ftp.c 13 Nov 2013 20:41:14 - 1.83
+++ ftp.c 29 Nov 2013 21:17:49 -
@@ -1089,9 +1089,10 @@
may_send_noop_char();
d = 0;
do {
- wr = write(fileno(fout), buf + d, rd);
- if (wr == -1 errno == EPIPE)
+ if ((wr = write(fileno(fout), buf + d, rd)) ==
-1) {
+ d = -1;
break;
+ }
d += wr;
rd -= wr;
} while (d c);
p is set to -1 to display the proper error message
of course here I meant 'd'
Re: Weird loop in ftp client
What about that?
Index: ftp.c
===
RCS file: /cvs/src/usr.bin/ftp/ftp.c,v
retrieving revision 1.83
diff -u -r1.83 ftp.c
--- ftp.c 13 Nov 2013 20:41:14 - 1.83
+++ ftp.c 29 Nov 2013 21:17:49 -
@@ -1089,9 +1089,10 @@
may_send_noop_char();
d = 0;
do {
- wr = write(fileno(fout), buf + d, rd);
- if (wr == -1 errno == EPIPE)
+ if ((wr = write(fileno(fout), buf + d, rd)) ==
-1) {
+ d = -1;
break;
+ }
d += wr;
rd -= wr;
} while (d c);
p is set to -1 to display the proper error message
Re: Weird loop in ftp client
If the disk is full, having time to retry was nice ?
Progress 90%.
Nop try again...
Anyway,
Warning on error would be a first i guess to spot REAL problem. (Without
changing the behavior).
[Mail] + Broken Pipe, WEEK END.
On Fri, Nov 29, 2013 at 2:44 PM, Maxime Villard m...@m00nbsd.net wrote:
What about that?
Index: ftp.c
===
RCS file: /cvs/src/usr.bin/ftp/ftp.c,v
retrieving revision 1.83
diff -u -r1.83 ftp.c
--- ftp.c 13 Nov 2013 20:41:14 - 1.83
+++ ftp.c 29 Nov 2013 21:17:49 -
@@ -1089,9 +1089,10 @@
may_send_noop_char();
d = 0;
do {
- wr = write(fileno(fout), buf + d, rd);
- if (wr == -1 errno == EPIPE)
+ if ((wr = write(fileno(fout), buf + d,
rd)) == -1) {
+ d = -1;
break;
+ }
d += wr;
rd -= wr;
} while (d c);
p is set to -1 to display the proper error message
--
-
() ascii ribbon campaign - against html e-mail
/\
Re: Weird loop in ftp client
On Sat, Nov 23, 2013 at 5:29 PM, Stuart Henderson st...@openbsd.org wrote: On 2013/11/23 14:39, sven falempin wrote: Hello, Another point of vue : Because curl is in base, what does ftp client add to the system ? ftp(1) is not without problems (bad ssl support, semi-broken proxy support, difficult code) but it has one major advantage: the small version used on ramdisks is quite small, something like 80k. cURL is not in base, but even if it was, it looks to be much too big for the ramdisks. my bad, i was really thinking it was in there -- - () ascii ribbon campaign - against html e-mail /\
Re: Weird loop in ftp client
On Sat, Nov 23, 2013 at 5:47 PM, Theo de Raadt dera...@cvs.openbsd.orgwrote: On 2013/11/23 14:39, sven falempin wrote: Hello, Another point of vue : Because curl is in base, what does ftp client add to the system ? ftp(1) is not without problems (bad ssl support, semi-broken proxy support, difficult code) but it has one major advantage: the small version used on ramdisks is quite small, something like 80k. cURL is not in base, but even if it was, it looks to be much too big for the ramdisks. As a note to anyone skilled... take the above issues as a challenge. If you can improve those problems in ftp(1), we'd love it. But it must remain a fairly small program. Proposed (incomplete) behavior : EBADF = fatal EFBIG, EIO, EPIPE = warn + quit this file ( but EPIPE = wait for someone to open the pipe ? or warn + quit this file ) ENOSPC, EDQUOT = warn + quit this file, or warn + sleep (hope someone rm some stuff) EINTR = info + retry ? how many retry ? or for how long ? EFAULT = call the martian + fatal EAGAIN = call poll and retry ? some fs can do that right ? Network related = ignore (fout is not a socket) But this one [EIO] The process is a member of a background process attempting to write to its controlling terminal, TOSTOP is set on the terminal, the process isn't ignoring the SIGTTOUT signal and the thread isn't blocking the SIGTTOUT signal, and either the process was created with vfork(2)http://www.openbsd.org/cgi-bin/man.cgi?query=vforksektion=2arch=i386apropos=0manpath=OpenBSD+Currentand hasn't successfully executed one of the exec functions or the process group is orphaned. Yes this one would require skillz of fatal(I dont get it, can you please use your ftp client like anyone else :-)); -- - () ascii ribbon campaign - against html e-mail /\
Re: Weird loop in ftp client
On Sun, Nov 24, 2013 at 12:11 PM, sven falempin sven.falem...@gmail.comwrote: On Sat, Nov 23, 2013 at 5:47 PM, Theo de Raadt dera...@cvs.openbsd.org wrote: On 2013/11/23 14:39, sven falempin wrote: Hello, Another point of vue : Because curl is in base, what does ftp client add to the system ? ftp(1) is not without problems (bad ssl support, semi-broken proxy support, difficult code) but it has one major advantage: the small version used on ramdisks is quite small, something like 80k. cURL is not in base, but even if it was, it looks to be much too big for the ramdisks. As a note to anyone skilled... take the above issues as a challenge. If you can improve those problems in ftp(1), we'd love it. But it must remain a fairly small program. Proposed (incomplete) behavior : EBADF = fatal EFBIG, EIO, EPIPE = warn + quit this file ( but EPIPE = wait for someone to open the pipe ? or warn + quit this file ) $ cat /dev/zero | cat /dev/null [1] 15546 28664 $ kill -INT 28664 $ jobs [1] + Broken pipe cat /dev/zero | Interruptcat /dev/null here, 15546 exits because of EPIPE during write it does not warn about anything 28664 could've been signaled in any fashion, INT is used to highlight it might be interactive use why should a user interrupt end up as a warning? big friggen deal just exit the program ENOSPC, EDQUOT = warn + quit this file, or warn + sleep (hope someone rm some stuff) EINTR = info + retry ? how many retry ? or for how long ? EFAULT = call the martian + fatal EAGAIN = call poll and retry ? some fs can do that right ? Network related = ignore (fout is not a socket) But this one [EIO] The process is a member of a background process attempting to write to its controlling terminal, TOSTOP is set on the terminal, the process isn't ignoring the SIGTTOUT signal and the thread isn't blocking the SIGTTOUT signal, and either the process was created with vfork(2) http://www.openbsd.org/cgi-bin/man.cgi?query=vforksektion=2arch=i386apropos=0manpath=OpenBSD+Current and hasn't successfully executed one of the exec functions or the process group is orphaned. Yes this one would require skillz of fatal(I dont get it, can you please use your ftp client like anyone else :-)); -- - () ascii ribbon campaign - against html e-mail /\
Re: Weird loop in ftp client
On Sun, Nov 24, 2013 at 7:29 PM, sven falempin sven.falem...@gmail.com wrote: On Sun, Nov 24, 2013 at 5:57 PM, Andres Perera andre...@zoho.com wrote: On Sun, Nov 24, 2013 at 12:11 PM, sven falempin sven.falem...@gmail.com wrote: On Sat, Nov 23, 2013 at 5:47 PM, Theo de Raadt dera...@cvs.openbsd.orgwrote: On 2013/11/23 14:39, sven falempin wrote: Hello, Another point of vue : Because curl is in base, what does ftp client add to the system ? ftp(1) is not without problems (bad ssl support, semi-broken proxy support, difficult code) but it has one major advantage: the small version used on ramdisks is quite small, something like 80k. cURL is not in base, but even if it was, it looks to be much too big for the ramdisks. As a note to anyone skilled... take the above issues as a challenge. If you can improve those problems in ftp(1), we'd love it. But it must remain a fairly small program. Proposed (incomplete) behavior : EBADF = fatal EFBIG, EIO, EPIPE = warn + quit this file ( but EPIPE = wait for someone to open the pipe ? or warn + quit this file ) $ cat /dev/zero | cat /dev/null [1] 15546 28664 $ kill -INT 28664 $ jobs [1] + Broken pipe cat /dev/zero | Interruptcat /dev/null here, 15546 exits because of EPIPE during write it does not warn about anything 28664 could've been signaled in any fashion, INT is used to highlight it might be interactive use why should a user interrupt end up as a warning? big friggen deal just exit the program because the job is unfinished, moreover you just kill one of two programs I do not think your case is relevant. the process getting EPIPE doesn't know that the process reading was killed it doesn't know anything other than EPIPE why woud it spin infinately waiting for the fd to open again, as you suggested? or even print a warning? ENOSPC, EDQUOT = warn + quit this file, or warn + sleep (hope someone rm some stuff) EINTR = info + retry ? how many retry ? or for how long ? EFAULT = call the martian + fatal EAGAIN = call poll and retry ? some fs can do that right ? Network related = ignore (fout is not a socket) But this one [EIO] The process is a member of a background process attempting to write to its controlling terminal, TOSTOP is set on the terminal, the process isn't ignoring the SIGTTOUT signal and the thread isn't blocking the SIGTTOUT signal, and either the process was created with vfork(2)http://www.openbsd.org/cgi-bin/man.cgi?query=vforksektion=2arch=i386apropos=0manpath=OpenBSD+Currentand hasn't successfully executed one of the exec functions or the process group is orphaned. Yes this one would require skillz of fatal(I dont get it, can you please use your ftp client like anyone else :-)); -- - () ascii ribbon campaign - against html e-mail /\ -- - () ascii ribbon campaign - against html e-mail /\
Re: Weird loop in ftp client
On Fri, Nov 22, 2013 at 10:04:02PM +0100, Maxime Villard wrote: Le 22/11/2013 17:48, Ted Unangst a ??crit : On Fri, Nov 22, 2013 at 10:09, Stuart Henderson wrote: On 2013/11/22 07:25, Maxime Villard wrote: If write() fails without EPIPE, d is decremented, and the function keeps looping. If write() succeeds after several loops, d will be negative, and the function will write from buf-XX. When does write() fail and do we want to keep writing? (If I fill up the filesystem, do I really want to spin here?) I must say that I actually fail to see why EPIPE is the only case handled; it should stop looping regardless of the errno code, shouldn't it? often you'll ned to retry on EAGAIN. But indeed, the code is weird. Shouldn't it be something more like this? Otherwise if the write() fails, we attempt writing one byte fewer for every retry. That looks better to me.
Re: Weird loop in ftp client
Hello, Another point of vue : Because curl is in base, what does ftp client add to the system ? Cheers :-) On Sat, Nov 23, 2013 at 5:40 AM, Otto Moerbeek o...@drijf.net wrote: On Fri, Nov 22, 2013 at 10:04:02PM +0100, Maxime Villard wrote: Le 22/11/2013 17:48, Ted Unangst a ??crit : On Fri, Nov 22, 2013 at 10:09, Stuart Henderson wrote: On 2013/11/22 07:25, Maxime Villard wrote: If write() fails without EPIPE, d is decremented, and the function keeps looping. If write() succeeds after several loops, d will be negative, and the function will write from buf-XX. When does write() fail and do we want to keep writing? (If I fill up the filesystem, do I really want to spin here?) I must say that I actually fail to see why EPIPE is the only case handled; it should stop looping regardless of the errno code, shouldn't it? often you'll ned to retry on EAGAIN. But indeed, the code is weird. Shouldn't it be something more like this? Otherwise if the write() fails, we attempt writing one byte fewer for every retry. That looks better to me. -- - () ascii ribbon campaign - against html e-mail /\
Re: Weird loop in ftp client
sven falempin sven.falempin at gmail.com writes: Hello, Another point of vue : Because curl is in base, what does ftp client add to the system ? pkg_delete curl and you'll understand.
Re: Weird loop in ftp client
On 11/23/13 20:39, sven falempin wrote: Hello, Another point of vue : Because curl is in base, what does ftp client add to the system ? 1. It's not. 2. Interactivity. /Alexander
Re: Weird loop in ftp client
On 2013/11/23 14:39, sven falempin wrote: Hello, Another point of vue : Because curl is in base, what does ftp client add to the system ? ftp(1) is not without problems (bad ssl support, semi-broken proxy support, difficult code) but it has one major advantage: the small version used on ramdisks is quite small, something like 80k. cURL is not in base, but even if it was, it looks to be much too big for the ramdisks.
Re: Weird loop in ftp client
On 2013/11/23 14:39, sven falempin wrote: Hello, Another point of vue : Because curl is in base, what does ftp client add to the system ? ftp(1) is not without problems (bad ssl support, semi-broken proxy support, difficult code) but it has one major advantage: the small version used on ramdisks is quite small, something like 80k. cURL is not in base, but even if it was, it looks to be much too big for the ramdisks. As a note to anyone skilled... take the above issues as a challenge. If you can improve those problems in ftp(1), we'd love it. But it must remain a fairly small program.
Re: Weird loop in ftp client
On 2013/11/22 07:25, Maxime Villard wrote:
Hi,
a thing I spotted some weeks ago
- - - usr.bin/ftp/ftp.c l.1090 - - -
d = 0;
do {
wr = write(fileno(fout), buf + d, rd);
if (wr == -1 errno == EPIPE)
break;
d += wr;
rd -= wr;
} while (d c);
If write() fails without EPIPE, d is decremented, and the function
keeps looping. If write() succeeds after several loops, d will be
negative, and the function will write from buf-XX.
Since d is later used to display a proper error message, I'd suggest
this patch:
Index: ftp.c
===
RCS file: /cvs/src/usr.bin/ftp/ftp.c,v
retrieving revision 1.83
diff -u -r1.83 ftp.c
--- ftp.c 13 Nov 2013 20:41:14 - 1.83
+++ ftp.c 22 Nov 2013 06:23:32 -
@@ -1094,7 +1094,7 @@
break;
d += wr;
rd -= wr;
- } while (d c);
+ } while (d c d 0);
if (rd != 0)
break;
bytes += c;
Any opinion?
Shouldn't it be something more like this? Otherwise if the write() fails,
we attempt writing one byte fewer for every retry.
Index: ftp.c
===
RCS file: /cvs/src/usr.bin/ftp/ftp.c,v
retrieving revision 1.83
diff -u -p -r1.83 ftp.c
--- ftp.c 13 Nov 2013 20:41:14 - 1.83
+++ ftp.c 22 Nov 2013 10:08:16 -
@@ -1090,10 +1090,13 @@ recvrequest(const char *cmd, const char
d = 0;
do {
wr = write(fileno(fout), buf + d, rd);
- if (wr == -1 errno == EPIPE)
- break;
- d += wr;
- rd -= wr;
+ if (wr == -1) {
+ if (errno == EPIPE)
+ break;
+ } else {
+ d += wr;
+ rd -= wr;
+ }
} while (d c);
if (rd != 0)
break;
Re: Weird loop in ftp client
On Fri, 22 Nov 2013, Stuart Henderson wrote:
do {
wr = write(fileno(fout), buf + d, rd);
- if (wr == -1 errno == EPIPE)
- break;
- d += wr;
- rd -= wr;
+ if (wr == -1) {
+ if (errno == EPIPE)
+ break;
+ } else {
+ d += wr;
+ rd -= wr;
+ }
} while (d c);
That still loops endlessly for errors other than EPIPE.
Re: Weird loop in ftp client
On Fri, Nov 22, 2013 at 10:09, Stuart Henderson wrote: On 2013/11/22 07:25, Maxime Villard wrote: If write() fails without EPIPE, d is decremented, and the function keeps looping. If write() succeeds after several loops, d will be negative, and the function will write from buf-XX. When does write() fail and do we want to keep writing? (If I fill up the filesystem, do I really want to spin here?) Shouldn't it be something more like this? Otherwise if the write() fails, we attempt writing one byte fewer for every retry. That looks better to me.
Re: Weird loop in ftp client
Le 22/11/2013 17:48, Ted Unangst a écrit : On Fri, Nov 22, 2013 at 10:09, Stuart Henderson wrote: On 2013/11/22 07:25, Maxime Villard wrote: If write() fails without EPIPE, d is decremented, and the function keeps looping. If write() succeeds after several loops, d will be negative, and the function will write from buf-XX. When does write() fail and do we want to keep writing? (If I fill up the filesystem, do I really want to spin here?) I must say that I actually fail to see why EPIPE is the only case handled; it should stop looping regardless of the errno code, shouldn't it? Shouldn't it be something more like this? Otherwise if the write() fails, we attempt writing one byte fewer for every retry. That looks better to me.
