eCos termios patch

Jonathan Larmour jifl@eCosCentric.com
Thu Jan 23 05:50:00 GMT 2003


Rick Richardson wrote:
> Jonathon:
> 
> I submitted this patch for termios to the eCos list back in December,
> and was wondering if it will be accepted.  There were no comments on it
> either way.


How very odd. I see it in the list archives, but I have no record of it 
and it's not like me to lose a patch. Really not like me. Wibble.

Anyway, as for the patch. In order of issue....

First of all ONLCR. Yes I see you're very right that that's broken. I 
think your fix could be improved though. Could you try the attached patch 
for this problem instead please? (Unified diff was unclear, so I made it a 
context patch). I'm afraid I haven't got the hardware setup right now to 
do so myself.

The reason is that the return value wasn't right anyway, and it made sense 
to conflate the two cyg_io_writes into one. I decided the variable names 
were unclear too, and could get rid of actually_written. Oh and a more 
recent standard to the one I had been using before clarifies that OPOST 
should be set for ONLCR to take effect.

Next: using termios_write for ECHO. Entirely correct.

Next: VMIN processing. Something's not quite right here. Here's the patch 
excerpt again:

1	if (t->c_cc[ VMIN ] ) {
2	    if ( dev_buf_conf.rx_count >= t->c_cc[ VMIN ] )
3		*len = min(*len, dev_buf_conf.rx_count);
4	    else if (*len >= t->c_cc[ VMIN ] )
5		*len = t->c_cc[ VMIN ];
6	    else {
7		// This case is not handled correctly yet; needs mods to
8		// the loop below to do it right.  We must wait for MIN
9		// chars, but return only *len of them.  As it is right
10		// now, we will wait for only *len chars to arrive.
11	    }
12	} else {
13	    *len = min(*len, dev_buf_conf.rx_count);
14	}

The problem is on line 4/5 that we should return more than VMIN bytes if 
VMIN bytes are already available from the device, up to a maximum of *len 
obviously. So capping *len at VMIN bytes isn't the right thing to do. So 
lines 4/5 shouldn't be there.

But in any case I've looked closer at handling VMIN and as I'm sure you've 
seen, there's this bit:

         } else { // non-canonical mode
             /* This is completely wrong:
             if ( size+1 >= t->c_cc[ VMIN ] )
             returnnow = true;*/
         } // else

I commented out these lines because someone on ecos-discuss told me it was 
all completely wrong. At the time I was too busy to argue and just did it, 
but now I'm not so sure. In fact I'm certain I've found the problem with 
it - it simply didn't check for MIN==0.

So based on all the above I've included my suggested patch for all this as 
an alternative to yours. You'll need to patch a fresh version of 
termiostty.c from CVS.

Let me know if it works for you. Sorry I wasn't able to test it myself.

Jifl
-- 
eCosCentric       http://www.eCosCentric.com/       <info@eCosCentric.com>
--[ "You can complain because roses have thorns, or you ]--
--[  can rejoice because thorns have roses." -Lincoln   ]-- Opinions==mine
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: termiosnlcr.pat.txt
URL: <http://sourceware.org/pipermail/ecos-patches/attachments/20030123/0d748273/attachment.txt>


More information about the Ecos-patches mailing list