Le 22/11/2013 11:09, Stuart Henderson a écrit :
> 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 -0000 1.83
>> +++ ftp.c 22 Nov 2013 06:23:32 -0000
>> @@ -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 -0000 1.83
> +++ ftp.c 22 Nov 2013 10:08:16 -0000
> @@ -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;
>
Yes but, if the write() always fails, there's an infinite loop here.
