From: corinna DOT vinschen AT cityweb DOT de (Corinna Vinschen)
Subject: Re: B20 patch: CR handling in fhandler_console
3 Dec 1998 03:27:03 -0800
Message-ID: <3666754A.E442D1BB.cygnus.cygwin32.developers@cityweb.de>
References: <3665D672 DOT A1722461 DOT cygnus DOT cygwin32 DOT developers AT cityweb DOT de>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
To: Christopher Faylor <cgf AT cygnus DOT com>, DJ Delorie <dj AT delorie DOT com>,
        cygwin32-developers AT cygnus DOT com

Christopher Faylor wrote:
> 
> On Wed, Dec 02, 1998 at 09:42:42PM -0500, DJ Delorie wrote:
> >Corinna Vinschen wrote:
> >> in fhandler_console::read1() a short piece of code
> >> ignores CR if it is the one and only char in console input.
> >>
> >> I have found no circumstance, in which this code makes sense.
> >
> >Be careful.  If the console is in text mode, the \r character
> >will be stripped (i.e. crlf->nl conversion).  If the user
> >asked to read one character, and it is stripped, the function
> >will report that zero characters were read.  This is an EOF
> >indicator, even though it is not really EOF.
> >
> >What the code should do is *if* the \r would be stripped,
> >it should read the following \n.  If it can tell that
> >the \r is alone (no \n) but would be stripped, it should
> >probably change it to a \n.  If the console is in binary
> >mode (no stripping), it can safely return a \r character.
> 
> At that point in the function, the console is *guaranteed* to be in cooked
> mode.  If it sees a \r, then it should be ignored.
> 
> I think the code is correct, but then I would, because I wrote it.

Hmm. I see it, but I can't believe it. I have two arguments
against it:

- The next if clause after the mentioned one is the code, which
  kills CR if LF follows. So, in this case, the mentioned one is
  unnecessary.

- I have a code example (from an implementation of att-robots):

        struct termio sav_tio, tio;

        tcgetattr (0, &sav_tio);
        tio = sav_tio;
        tio.c_lflag &= ~(ECHO | ICANON);
        tio.c_cc[VMIN] = 1;
        tio.c_cc[VTIME] = 0;
        tcsetattr(0, TCSADRAIN, &tio);

		... player plays its game ...
		... all input is done by read(0, ...

        tcsetattr(0, TCSADRAIN, &sav_tio);
	
		"Hit [RETURN] to continue"

	getchar();

  The above getchar() fails, if the player presses RETURN without
  pressing any other key before! This behaviour is not only incorrect,
  but also doesn't happen in CYGWIN=tty mode.
  An STRACE output shows, that ReadFile only returns CR, which
  is discarded by the mentioned code piece. If the player presses
  e.g. SPACE RETURN, you can see: 0x20, 0x0d as input, which is
  _not_ cooked by the code in read1. I think, this is also not, what
  the code should be for.
  Maybe, this can be backtracked to an error in Windows console handling,
  but IMHO the console code shouldn't support this misbehaviour.

These are _not_ arguments for the correctness of the patch. But I'm
testing it and anything works as before, moreover, the above code
works, too. So, the patch could be a temporary workaround.

Eventually, the next if ("Perform a similar test on a buffer with > 1
character.  If it ends in \r\n then fix it up") should be deleted, too,
because this conversions have to be done by fhandler_console::read().
This method contains the conversions, which are configured by stty modes
(e.g. ICRNL, IUCLC). Cooking is the business of configurable code. The
discussed hardcoded cooking code in read1() could break the stty settings.
What, if ICRNL is unset by intention?

Regards,
Corinna