search

david-maw / StreamSSL

The StreamSSL sample described in CodeProject
Other
47 stars 24 forks source link

CPassiveSock should be synced with CActiveSock #52

Closed 0ric1 closed 5 years ago

0ric1 commented 5 years ago

I have done changes mainly in CActiveSock because I only needed the client side. In CPassiveSock seems to be a lot of same code as in CActiveSock so maybe a common CBaseSock class should handle this or the changed code from CActiveSock should be merged into CPassiveSock.

david-maw commented 5 years ago

Agreed; I'll start a CBaseSock implementation.

0ric1 commented 5 years ago

Cool, I'll commit more changes tomorrow but not to CActive/PassicSock, so we get no conflict.

david-maw commented 5 years ago

ok, I started it and created a pull request "Implement BaseSock #54" but I couldn't figure out how to request a review from you (I thought "reviewers" would be it, but evidently I can't drive it)! If you wouldn't mind taking a look to see if there's anything obviously wrong before I march too far down a dead end road that'd be great.

0ric1 commented 5 years ago

I merged your branch into my local master and successfully compiled it. Didn't find anything obviously wrong.

david-maw commented 5 years ago

ok, I merged that and created more with pull request #60. With that in place I think all the major issues are gone. I still have to reconcile a few remaining methods but (like the two definitions of "Disconnect") but as far as I can tell it all works.

david-maw commented 5 years ago

I've resolved the remaining issues and merged the changes into master