[cl-plus-ssl-devel] Two LISTEN bugs

David Lichteblau david at lichteblau.com
Wed May 12 07:04:50 UTC 2010


Hi there,

just a couple of comments:

Quoting Ron Garret (ron at flownet.com):
> > NOTE: This has only been tested on CCL.

Have you written test cases for this?  Please submit the test cases,
too.

Our existing test suite in test.lisp isn't that bad, really, it just
doesn't cover the case you're seeing, so let's fix that.

In particular, have you checked whether it passes the test case
submitted by Mr. gzip4?  (I'm guessing not, since that one was
SBCL-specific.  Let's make it as portable as possible, please.)

> > ; Bug fix: stream-listen used to hang rather than return nil when no input
> > (defmethod stream-listen ((stream ssl-stream))
> >  (or (ssl-stream-peeked-byte stream)
> >      (setf (ssl-stream-peeked-byte stream)
> >            (let ((buf (ssl-stream-input-buffer stream)))
> >              (with-pointer-to-vector-data (ptr buf)
> >                (let ((n (ssl-read (ssl-stream-handle stream) ptr 1)))
> >                  (and (> n 1) (buffer-elt buf 0))))))))

I haven't reviewed this carefully, but isn't there error handling
lacking in the 0 case?

> Oops, should be (> n 0) in the last line.

See what I mean about test case? ;-)
That's another nice situation to check for right there.


Thanks for the code though.

In general, lazy (or busy) maintainers like me are more likely to commit
things when they just "look right". i.e. if something comes with so many
tests that obviously more thought has gone into it than I could offer
through a quick review anyway, I might as well commit it.

BTW, a unified diff would be nice rather than copy&paste.


d.




More information about the cl-plus-ssl-devel mailing list