Subject: [clisp-list] bugs in (setf (stream-element-type x) ...)

From: Todd Sabin <[email-masked]>
Date: Sat Dec 29 08:11:03 2001

Hi,

This is with clisp-2.27, on RH Linux 6.2.

So, I was trying to write some sample code that would act as an HTTP
client.  What I did was (socket-connect ...), (format ...), then
(read-line ...) until I got the empty line.  At that point, I did
(setf (stream-element-type *socket*) '(unsigned byte 8)), and then
(read-sequence *array* *socket*), where *array* was obtained from
make-array with the size specified by the Content-Length: header.

Everything worked, except that the first element in *array* got the
value 10 (LF), and the rest of the data was shifted by one byte,
leaving one byte of the actual data unread.  I realized that somehow I
was getting the final LF from the headers, even though it should have
already been read.  Indeed, if instead of doing the (setf
(stream-element-type ...)), I just did (read-char *socket*), I would
get the first char of the data, and not #\Newline.

I figured that it probably had something to do with CRLF -> #\Newline
translation, and that maybe it wouldn't be a problem with buffering.
So, I tried:

[1]> (setq *my-sock* (socket-connect 80 "jetcar.qnz.org" :buffered t))
#<IO BUFFERED SOCKET-STREAM CHARACTER jetcar.qnz.org:80>
[2]> (format *my-sock* "GET /icons/apache_pb.gif HTTP/1.0

")
NIL
[3]> (force-output *my-sock*)
NIL
[4]> (read-line *my-sock*)
"HTTP/1.1 200 OK" ;
NIL
[5]> (setf (stream-element-type *my-sock*) '(unsigned-byte 8))

*** - SYSTEM::BUILT-IN-STREAM-SET-ELEMENT-TYPE: argument 
*** - handle_fault error2 ! address = 0x40490824 not in [0x2029E000,0x2035A410) !
SIGSEGV cannot be cured. Fault address = 0x40490824.
Segmentation fault (core dumped)

Well, I tracked the crash down, and this patch fixes it:

--- stream.d	Mon Jul 16 10:35:54 2001
+++ ../../clisp-2.27-tas/src/stream.d	Fri Dec 28 23:31:44 2001
@@ -16238,9 +16238,9 @@
       #ifdef SOCKET_STREAMS
       case strmtype_twoway_socket:
         # Apply to the input and output side individually.
-        pushSTACK(TheStream(STACK_1)->strm_twoway_socket_input); pushSTACK(STACK_(0+1));
+        pushSTACK(TheStream(stream)->strm_twoway_socket_input); pushSTACK(STACK_(0+1));
         funcall(L(built_in_stream_set_element_type),2);
-        pushSTACK(TheStream(STACK_1)->strm_twoway_socket_output); pushSTACK(STACK_(0+1));
+        pushSTACK(TheStream(stream)->strm_twoway_socket_output); pushSTACK(STACK_(0+1));
         funcall(L(built_in_stream_set_element_type),2);
         break;
       #endif

I'm not sure why the code was using STACK_1 instead of stream to begin
with, but if there was a reason to reference the stack directly, it
should have been STACK_2, not STACK_1.  With the patch, the buffered
case works as expected, no extra LF.

The problem in the unbuffered case turns out to be the way CRLF is
handled in the unbuffered case.  If read-line encounters a CRLF in the
input stream, the LF remains unread, and the ignore_next_LF flag is
set.  When (setf (stream-element-type ...)) is used, that flag is lost
or discarded, which causes the problem.

There seem to be a few ways around it, but I haven't attempted any of
them.

 o The flag could be done away with by reading the next char from
 input, and discarding it if it's a LF, and unreading it if not.  In
 theory, this might cause an app to hang where it wouldn't have before,
 but I think the risk of that is pretty low.

 o The flag could be preserved across the (setf (stream-element-type
 ...)), but then all the read-byte functions would need to be aware of
 it.

 o apps can do a (peek-char *stream*) before
 (setf (stream-element-type ...)) to workaround it.  Seems gross to
 require that of apps, though.

Anyway, this is the first I've looked at clisp source code, so I'm
sure you'll have a better idea of how to handle this.

Todd

p.s.  I looked, but didn't find, some instructions for developers on
how to debug clisp.  I managed to build it with debugging info by
adding the appropriate "-g"s to the Makefile, but it crashed with
sigsegv whenever I tried to do a step or next in gdb.  Is there a
secret to debugging clisp in gdb?  I had to resort to printfs.  I saw
the Makefile.devel, but it didn't seem to have any support for doing a
debug build.

p.p.s.  Is there any way not to do the CRLF translation?  As it
stands, if I wanted to, e.g., write a network server that verified
strict compliance of its clients, i.e., make sure there are both a CR
and a LF at the end of lines, there's no way to do it with clisp,
unless I treat everything as bytes instead of chars, which makes lots
of other things unwieldy.

---

From: Sam Steingold <[email-masked]>
Date: Tue Jan 1 20:29:02 2002

> * In message <[email-masked]>
> * On the subject of "[clisp-list] bugs in (setf (stream-element-type x) ...)"
> * Sent on 29 Dec 2001 11:10:47 -0500
> * Honorable Todd Sabin <[email-masked]> writes:
>
you mean (format *my-sock* "GET /icons/apache_pb.gif HTTP/1.0~2%") ? :-)
> +++ ../../clisp-2.27-tas/src/stream.d	Fri Dec 28 23:31:44 2001

thanks for the patch - I applied it.

>  but I think the risk of that is pretty low.

using UnbufferedStreamLow_listen() should prevent hanging.

        if (ignore_next_LF) {
          if (UnbufferedStreamLow_listen()) {
            if (peak_char() == LF)
              read_char();
          }
        }

patch would be welcome.

> the Makefile.devel, but it didn't seem to have any support for doing a

this is strange - I have never seen this.
did you try giving "debug" arg to makemake?
(makemake --help mentions this)

> of other things unwieldy.

This is true.
This feature has been requested in the past and was turned down - it
adds more complexity for a doubtful benefit.
The real answer is: "if you want to distinguish between CR and CR/LF,
you are doing something too low level" (or, maybe even "if you think you
need to distinguish between CR and CR/LF, think again").

If you implement a clean interface (e.g., adding INPUT-LINE-TERMINATOR
slot to ENCODING) and send us a patch, we will think about it.

-- 
Sam Steingold (http://www.podval.org/~sds)
Keep Jerusalem united! <http://www.onejerusalem.org/Petition.asp>
Read, think and remember! <http://www.iris.org.il> <http://www.memri.org/>
"Syntactic sugar causes cancer of the semicolon."	-Alan Perlis

---

From: Todd Sabin <[email-masked]>
Date: Wed Jan 2 08:31:28 2002

> you mean (format *my-sock* "GET /icons/apache_pb.gif HTTP/1.0~2%") ? :-)

No, there were CR returns in there, as well.  I suppose I could have
done (format *my-sock* "GET /icons/apache_pb.gif HTTP/1.0^M~%^M~%").
Most ascii based network protocols specify that lines should end with
CRLF, not just LF.  Many clients get this wrong, and most servers are
forgiving when clients get it wrong, but I like to get it right,
anyway.

>           if (UnbufferedStreamLow_listen()) {

Will the low-level peek_char do roughly the same thing as the
lisp-level one?  I.e., read-char, unread-char?  If so, then that may
not work, because it probably means the user can't unread the
#\Newline he's about to get, since we may have just unread the next
char.  Or is there a way around this?

Also, this wouldn't handle the case where the next character will be a
LF, but it's not available, yet.  This can happen in the buffered
case, as well, and it's not even necessary to use
(setf (stream-element-type ..)) to get incorrect results in that case.
It seems that the buffered case needs to pay attention to ignore_next_LF,
too.

There seem to be other implications with the way things currently
stand, as well.  Say I want to write an HTTP proxy.  What I'd do is
listen for a connection on a socket.  When I get one, do read-line
until I get an empty line, process the request, send the results to my
client and close the socket (stream).  If I do that but don't use a
buffered stream, then I'll end up sending a TCP RST to my client
because the last LF of his request will remain unread.  Ugh.

> (makemake --help mentions this)

Ah, didn't see makemake, thanks.  I just scanned though configure and
Makefile.

> need to distinguish between CR and CR/LF, think again").

Well, if the feature has been requested before and is being requested
again, perhaps that means there is a benefit.  I know why I'd want to
be able to distinguish CR and CRLF, and I even gave an example.

> slot to ENCODING) and send us a patch, we will think about it.

Well, I don't need it enough to implement it, presently.  Especially
if I'm not sure the maintainers are receptive to the idea.  Just
getting CRLF->LF translation right seems like enough of a challenge
for now.  I may just workaround it with peek-char and/or buffering,
but it'd be nice to fix it right.

Todd

---

From: Sam Steingold <[email-masked]>
Date: Thu Jan 3 07:46:08 2002

> * In message <[email-masked]>
> * On the subject of "Re: [clisp-list] bugs in (setf (stream-element-type x) ...)"
> * Sent on 02 Jan 2002 11:29:55 -0500

if you do
(socket-connect 80 "jetcar.qnz.org" :buffered t :external-format :dos)
then TERPRI will send CRLF.

> lisp-level one?  I.e., read-char, unread-char?

no, they use non-blocking libc routines.

> because the last LF of his request will remain unread.  Ugh.

yes, this is an issue.
patch is welcome.

> but it'd be nice to fix it right.

we _are_ receptive to good patches, especially when people are prepared
to stand by them and fix problems they might introduce.

-- 
Sam Steingold (http://www.podval.org/~sds)
Keep Jerusalem united! <http://www.onejerusalem.org/Petition.asp>
Read, think and remember! <http://www.iris.org.il> <http://www.memri.org/>
Lisp: its not just for geniuses anymore.

---

From: "John Wiseman" <[email-masked]>
Date: Sat Jan 5 13:13:02 2002

> make-array with the size specified by the Content-Length: header.

Seriously, you should never use read-line for that stuff, even if it
seems to "work".  HTTP (and many other protocols) use CRLF.  read-line
has nothing to do with CRLF.  It might work on your machine-of-the-
moment, in your lisp-of-the-moment, but it is wrong.

This is what I use in ACL (windows, linux) and MCL:

(defparameter *cr* (code-char 13))
(defparameter *lf* (code-char 10))

(defun read-telnet-line (stream)
  "Read a CRLF-terminated line"
  (let ((line (make-array 10 :element-type 'character :adjustable t
:fill-pointer 0))
        (char nil))
    (do () ((or (eq :eof (setq char (read-char stream NIL :eof)))
                (and (eq char *cr*) (eq (peek-char nil stream) *lf*)))
            (when (not (eq char :eof)) (read-char stream))
            line)
      (vector-push-extend char line))))

I haven't tried this in clisp.

John Wiseman

---

From: "John Wiseman" <[email-masked]>
Date: Sat Jan 5 13:20:02 2002

:fill-pointer 0))
>       (vector-push-extend char line))))

Eek, was I really using eq to compare characters?  This should, of course,
be

                 (and (eql char *cr*) (eql (peek-char nil stream) *lf*)))

John

---

From: Sam Steingold <[email-masked]>
Date: Sat Jan 5 17:22:03 2002

> * In message <[email-masked]>
> * On the subject of "RE: [clisp-list] bugs in (setf (stream-element-type x) ...)"
> * Sent on Sat, 5 Jan 2002 15:12:30 -0600
> * Honorable "John Wiseman" <[email-masked]> writes:
>
> Todd Sabin wrote:
> 

you mean it is not portable?  sure.

but there is nothing wrong with making CLISP handle this correctly, and
making encodings specify input line termination mode is, IMO, TRW.

wanna do it?

-- 
Sam Steingold (http://www.podval.org/~sds)
Keep Jerusalem united! <http://www.onejerusalem.org/Petition.asp>
Read, think and remember! <http://www.iris.org.il> <http://www.memri.org/>
Daddy, what does "format disk c: complete" mean?

---

From: "Johannes Grødem" <[email-masked]>
Date: Sat Jan 5 20:15:04 2002

* "John Wiseman" <[email-masked]>:

> Eek, was I really using eq to compare characters?

Isn't that okay?  I mean, for what reasons should (eq #\a #\a) be
nil?

(Just curious.)

-- 
johs

---

From: "John Wiseman" <[email-masked]>
Date: Sat Jan 5 21:25:01 2002

Johannes Grodem wrote:

> Isn't that okay?  I mean, for what reasons should (eq #\a #\a)
> be nil?

From the Hyperspec entry on eq
(http://www.xanalys.com/software_tools/reference/HyperSpec/Body/fun_eq.html)
:

  An implementation is permitted to make ``copies'' of characters and
  numbers at any time. The effect is that Common Lisp makes no
  guarantee that eq is true even when both its arguments are ``the
  same thing'' if that thing is a character or number.

John Wiseman

---

From: Todd Sabin <[email-masked]>
Date: Sun Jan 6 13:16:02 2002

> moment, in your lisp-of-the-moment, but it is wrong.

That depends on what you mean by 'wrong'.  And whether anything can
actually be considered 'right'.  I'm fairly new to doing sockets and
streams in Common Lisp, and have been trying to figure out the way
you're supposed to do stuff like this.  I haven't been able to figure
it out, yet.  If you can explain it to me, I'd be grateful.

> I haven't tried this in clisp.

If you do, I think you'll find that it doesn't work.  For character
streams, clisp automatically transforms CRLF -> LF, so you'll never
see a CR followed by a LF, unless the bytes have CRCRLF.  Furthermore,
I can't see that this translation is wrong.  The only thing I found on
it in the HyperSpec was that for character streams implementations are
supposed to do the 'appropriate' line termination transforms, or
something to that effect.  If you can show otherwise, I'd be
interested.  (Not that I like the CRLF -> LF transformation; I've
found it to be more trouble than it's worth, so far.)

Also, assuming you've read chars up to the point where the HTTP
headers end, how are you going to read the bytes, assuming that it's
binary data?  Nothing says that read-byte is supposed to work on
character streams, as far as I can tell, so it seems you either need
something non-portable like (setf (stream-element-type ...)), or an
implementation that allows you to mix bytes and chars, or you need a
guarantee from your implementation that CHARACTER is the same as
(unsigned-byte 8), and it doesn't do any CRLF -> LF type of tranform.

It seems the only truly portable approach is to always use read-byte
with (unsigned-byte 8) and write your own read line function which
does the transforms from bytes to chars for you.  If you do that,
though, you can't use FORMAT for output anymore.  I suppose you could
(format nil ...) and then map char-code over the resulting string and
write-byte them all.

At least, that's how things appear to my admittedly cl-inexperienced
eye.  Is there a less elaborate approach that's both considered
'right' (or at least 'good enough') and actually works in all
conforming implementations?

Not trying to be a pedant.  I'm really interested.

Todd

---

From: Todd Sabin <[email-masked]>
Date: Sun Jan 6 13:31:03 2002

> then TERPRI will send CRLF.

Ah, ok.

> no, they use non-blocking libc routines.

My point was that using peek_char in C will cause visible (incorrect)
changes at the lisp level.  Assume the data contains CR <non-LF>.  Now
if the user does (read-char stream), and then tries (unread-char CR),
it would cause an error, because the C code did a peek_char
(read,unread) during the read-char, so the last char read is not
really CR, but the <non-LF>.  The C code would need to account for
this case, and I don't see a straightforward way of doing it.

> > because the last LF of his request will remain unread.  Ugh.

Again, I don't see a nice way of handling it.  I guess you could check
for the LF just before closing, or doing a (setf (stream-element-type
...))  and eat it at that point.

Todd

---

From: [email-masked] (Don Cohen)
Date: Sun Jan 6 13:42:01 2002

> it out, yet.  If you can explain it to me, I'd be grateful.
For dealing with standard internet protocols I long ago gave up trying
to use characters in clisp and moved to bytes.  At least for reading.
If you have a string that you want to write then it's ok to write it
as characters.  My impression is that others who have tried to do
similar tasks have generally followed the same path, sometimes after
much effort trying to make it all work with characters.

You might also save a lot of time by using code that others have
already written for such things.  I recall a number of http servers.
Mine is in clocc under src/donc I believe (can't reach clocc just
now).  I've not tried to keep track of others but I recall seeing 
something about that not too long ago on this list.

---

From: Sam Steingold <[email-masked]>
Date: Sun Jan 6 16:55:02 2002

> * In message <[email-masked]>
> * Sent on Sun, 6 Jan 2002 13:39:13 -0800
> * Honorable [email-masked] (Don Cohen) writes:
>

> to use characters in clisp and moved to bytes.  At least for reading.

this is unfortunate.
what is the reason?
maybe we could patch CLISP to your tastes? :-)

-- 
Sam Steingold (http://www.podval.org/~sds)
Keep Jerusalem united! <http://www.onejerusalem.org/Petition.asp>
Read, think and remember! <http://www.iris.org.il> <http://www.memri.org/>
There is Truth, and its value is T.  Or just non-NIL.  So 0 is True!

---

From: Sam Steingold <[email-masked]>
Date: Sun Jan 6 17:23:01 2002

> * In message <[email-masked]>
> * Sent on 06 Jan 2002 16:30:35 -0500

the C code is very involved.
CLISP has a lot of functionality for non-blocking i/o (see
<http://cvs.sourceforge.net/cgi-bin/viewcvs.cgi/*checkout*/clisp/clisp/doc/impnotes.html?rev=HEAD&content-type=text/html#listen>
and below).
study the code and you will see how CLISP avoids these issues.

> ....))  and eat it at that point.

I already told you how to handle this nicely:
now MAKE-ENCODING
<http://cvs.sourceforge.net/cgi-bin/viewcvs.cgi/*checkout*/clisp/clisp/doc/impnotes.html?rev=HEAD&content-type=text/html#make-encoding>
takes :LINE-TERMINATOR keyword argument.
If you make it take two separate arguments instead:
:INPUT-LINE-TERMINATOR (new) and :OUTPUT-LINE-TERMINATOR (renamed from
:LINE-TERMINATOR), and make all i/o functions respect this change, and
make :OUTPUT-LINE-TERMINATOR default to :UNIX (as it does now), and
make :INPUT-LINE-TERMINATOR default to T (the current behavior: accept
either :UNIX, :MAC or :DOS), but able to take any other of the 3
keyword values.

-- 
Sam Steingold (http://www.podval.org/~sds)
Keep Jerusalem united! <http://www.onejerusalem.org/Petition.asp>
Read, think and remember! <http://www.iris.org.il> <http://www.memri.org/>
Those who value Life above Freedom are destined to lose both.

---

From: [email-masked] (Don Cohen)
Date: Sun Jan 6 17:49:04 2002

> maybe we could patch CLISP to your tastes? :-)

I don't know whether it's unfortunate.  

As I recall, there were two reasons.  One was the end of line issue
and the other was errors with character sets that had no character
defined for the bytes that came in.  Notice that when you write a 
server you don't have a lot of control over the bytes that others
send you.

I recall being told, (just as you said recently on this list), that if
you want to deal with CR and LF as different things then you're
working at too low a level for characters.  I suppose it's possible
that enough stuff has been added over the last few years that I could
now do all I want with characters, but I recall that I did waste (in
retrospect) a lot of time trying and failing.

My original code using characters did work in Allegro CL, but I think
that's because the default "character" set there was actually no
different from bytes, i.e., there were 256 of them, each represented
by a different 8 bit byte.  Perhaps clisp has such a character set,
but it was at least not the default when I was trying to write this
code.  In fact I recall trying different character sets.

Perhaps others who have written such software can confirm or refute
my impression that both the problem and solution were widespread.

---

From: Todd Sabin <[email-masked]>
Date: Sun Jan 6 23:48:08 2002

> study the code and you will see how CLISP avoids these issues.

We seem to be talking past each other.  I'm not talking about how to
avoid hanging.  I'm saying that the strategy of peeking the next char
when encountering a CR will not work because it breaks unread-char,
not because it might hang.  Anyway, the
:INPUT-LINE-TERMINATOR approach would make this go away, too.

> either :UNIX, :MAC or :DOS), but able to take any other of the 3

Shouldn't it also still accept :LINE-TERMINATOR for a while to avoid
breaking existing code?

If I have time, I'll look at doing this.

Todd