Re: bin/59587: ftp client ascii transfers with progress report fail

[Luke Mewburn <luke%mewburn.net@localhost>]

On 26-01-15 03:51, Taylor R Campbell wrote:
  | > Module Name:    src
  | > Committed By:   lukem
  | > Date:           Sat Dec  6 06:20:23 UTC 2025
  | > 
  | > Modified Files:
  | >         src/usr.bin/ftp: extern.h ftp.c util.c version.h
  | > 
  | > Log Message:
  | > ftp: fix ascii transfers with progress bar
  | > 
  | > Handle stdio interruption by signals and improve error handling
  | > in getc() and putc() on the control and data channels.
  | > Provide ftp_getc() and ftp_putc() wrappers that:
  | > - Retry the operation on EINTR or EAGAIN instead of failing.
  | > [...]
  | > +	while ((res = getc(fin)) == EOF) {
  | > +		if (feof(fin))
  | > +			break;		/* return EOF */
  | > +		if (ferror(fin)) {
  | > +			if ((errno == EINTR) || (errno == EAGAIN)) {
  | > +					/* retry on EINTR or EAGAIN */
  | > +				clearerr(fin);
  | > +				continue;
  | 
  | Why do you loop on EAGAIN?
  | 
  | Is the underlying file descriptor non-blocking?
  | 
  | => If it is non-blocking, then you presumably need to wait in
  |    select/poll for input to arrive -- otherwise this is a busy-wait.
  | 
  | => If it isn't non-blocking, then you should never get EAGAIN here --
  |    EAGAIN is generally only for non-blocking I/O calls to report that
  |    they can't perform the requested operation until you wait in
  |    select/poll for something to become ready (some input data to
  |    become ready or some output data to be processed from the buffer).
  | 
  | Or is there a bug somewhere that causes some syscall or library
  | routine involved to return EAGAIN on a signal, when it should really
  | either restart or return EINTR?  (I have never seen such a bug.)

I was following the pattern used previously in the ftp code for
handling interrupted raw I/O on the data and control sessions,
which may be on non-blocking descriptors.
For purity purposes I /could/ remove the EAGAIN checks, but I think
it makes it easier when eyeball checking the differences in
I/O handling across the code base for raw vs stdio.


To be frank, whilst a fix removing all use of non-signal-safe stdio
with ftp-specific code for the line protocol handling (FTP control,
HTTP headers) might be "better", I don't think it's worth the effort.
This works.

ftp as a protocol is on life support.  Whilst I've spent many years
augmenting this tool, it's a lost cause. HTTP "won". Other tools exist.
So aiming for perfection is not going to happen.
The whole code base started on 1980s grad student C code with too many
globals, setjmp, etc, and modified by 1990s grad students (me),
and others since.  It's not how anyone would write code today.