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

[Ron Garret]

I'm not very familiar with the code, so it's probably better if someone reviewed my changes regardless.  But I'll try to find some time to write a test and generate a diff.  Just FYI, CL+SSL out of the box fails 4 of 8 tests on the CCL trunk.  (My patch seems to have fixed one of them.  I now fail only 3 of 8.)

rg

On May 12, 2010, at 12:04 AM, David Lichteblau wrote:

> 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.