Re: ftp doesn't close(2) the output file

[Sunil Nimmagadda]

Theo de Raadt  writes:

>> > Index: fetch.c
>> > ===
>> > RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
>> > retrieving revision 1.161
>> > diff -u -p -r1.161 fetch.c
>> > --- fetch.c28 Feb 2017 06:31:12 -  1.161
>> > +++ fetch.c1 Mar 2017 23:21:46 -
>> > @@ -1041,6 +1041,8 @@ cleanup_url_get:
>> >fclose(fin);
>> >else if (s != -1)
>> >close(s);
>> > +  if (out >= 0 && out != fileno(stdout))
>> > +  close(out);
>> >free(buf);
>> >free(proxyhost);
>> >free(proxyurl);
>> 
>> I was wondering if the "out >= 0" condition is necessary as open(2)
>> errors are handled anyway.
>
> Sure, but it is cleaner to not call close(-1).

oh right, the cleanup_url_get ends up here as well, and this condition is
required. Sorry for the noise.

Re: ftp doesn't close(2) the output file

[Theo de Raadt]

> > Index: fetch.c
> > ===
> > RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
> > retrieving revision 1.161
> > diff -u -p -r1.161 fetch.c
> > --- fetch.c 28 Feb 2017 06:31:12 -  1.161
> > +++ fetch.c 1 Mar 2017 23:21:46 -
> > @@ -1041,6 +1041,8 @@ cleanup_url_get:
> > fclose(fin);
> > else if (s != -1)
> > close(s);
> > +   if (out >= 0 && out != fileno(stdout))
> > +   close(out);
> > free(buf);
> > free(proxyhost);
> > free(proxyurl);
> 
> I was wondering if the "out >= 0" condition is necessary as open(2)
> errors are handled anyway.

Sure, but it is cleaner to not call close(-1).

Re: ftp doesn't close(2) the output file

[Sunil Nimmagadda]

Stuart Henderson  writes:

> ftp doesn't close the output file after writing it. Normally you're
> exiting anyway at that point so it doesn't really matter, but if you've
> specified multiple URLs on the command line this leaks 1 FD per
> requested file. Most noticable if you do some lazy benchark like
> "ftp -o/dev/null `yes $SOMEURL | head -1000`" and run yourself out
> of FDs.
>
> Possible fix?
>
> Index: fetch.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
> retrieving revision 1.161
> diff -u -p -r1.161 fetch.c
> --- fetch.c   28 Feb 2017 06:31:12 -  1.161
> +++ fetch.c   1 Mar 2017 23:21:46 -
> @@ -1041,6 +1041,8 @@ cleanup_url_get:
>   fclose(fin);
>   else if (s != -1)
>   close(s);
> + if (out >= 0 && out != fileno(stdout))
> + close(out);
>   free(buf);
>   free(proxyhost);
>   free(proxyurl);

I was wondering if the "out >= 0" condition is necessary as open(2)
errors are handled anyway.

ftp doesn't close(2) the output file

[Stuart Henderson]

ftp doesn't close the output file after writing it. Normally you're
exiting anyway at that point so it doesn't really matter, but if you've
specified multiple URLs on the command line this leaks 1 FD per
requested file. Most noticable if you do some lazy benchark like
"ftp -o/dev/null `yes $SOMEURL | head -1000`" and run yourself out
of FDs.

Possible fix?

Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.161
diff -u -p -r1.161 fetch.c
--- fetch.c 28 Feb 2017 06:31:12 -  1.161
+++ fetch.c 1 Mar 2017 23:21:46 -
@@ -1041,6 +1041,8 @@ cleanup_url_get:
fclose(fin);
else if (s != -1)
close(s);
+   if (out >= 0 && out != fileno(stdout))
+   close(out);
free(buf);
free(proxyhost);
free(proxyurl);