Need to remove read() calls from escape sequence handling

[GoogleCodeExporter]

In addition to preventing a change from reading one byte per read() to
reading larger blocks for performance, the read() calls in the escape
sequence logic are potential points of failure.

The file descriptor for the pty is in non-blocking mode, so when we hit the
end of the available bytes the next read() reads 0 bytes and sets EAGAIN.
None of the read() calls in the escape sequence handlers are checked, so I
can't imagine we do the right thing if we get an EAGAIN in the middle of an
escape sequence.

I will work out an explicit state machine to do escape sequences robustly.

Original issue reported on code.google.com by bfrobin...@gmail.com on 22 Aug 2008 at 5:01

[GoogleCodeExporter]

State diagram in Graphviz format.
The character ranges for Esc [ control sequences are based on my own reading of 
the
Wikipedia article; handling of Esc ] OS commands is based on the existing code.

Original comment by bfrobin...@gmail.com on 22 Aug 2008 at 6:16

Attachments:

• escape.dot

[GoogleCodeExporter]

Ben, can you put in the command needed to render that graph file via graphviz?  
I
installed it but I can't figure out how to use it...

Original comment by jpsut...@gmail.com on 22 Aug 2008 at 10:44

[GoogleCodeExporter]

I used 'dot -Tpng escape.dot | display' where display is the one from 
ImageMagick.

Original comment by bfrobin...@gmail.com on 22 Aug 2008 at 11:17

[GoogleCodeExporter]

Thanks.  Got it.

Were you planning on re-implementing and replacing the existing code?

Original comment by jpsut...@gmail.com on 25 Aug 2008 at 1:05

[GoogleCodeExporter]

Yes. The work is in progress.

Original comment by bfrobin...@gmail.com on 25 Aug 2008 at 1:35

[GoogleCodeExporter]

OK.  I didn't want to start working on it and duplicate coding efforts :)

Original comment by jpsut...@gmail.com on 25 Aug 2008 at 1:38

[GoogleCodeExporter]

Every time I study another piece of the old code to learn what the behavior is, 
I
find more of what I can only assume to be schroedinbugs.
Lesson learned: try to (at least temporarily) understand each new piece of code 
that
gets committed. It's better to debug new code than old code.

Original comment by bfrobin...@gmail.com on 27 Aug 2008 at 12:43

[GoogleCodeExporter]

Well, I had to go look up "schroedinbugs" just to find out what it means.  I'll 
take
that thought as an indication that I have well obfuscated my implementation of 
the
escape sequence / OS command handling (though unintentionally, I assure you :) 
).

Original comment by jpsut...@gmail.com on 27 Aug 2008 at 1:10

[GoogleCodeExporter]

Remodeling completed in r157. Plenty of new code to review and test.

Original comment by bfrobin...@gmail.com on 27 Aug 2008 at 3:07

• Changed state: Fixed