search

fpagliughi / sockpp

Modern C++ socket library.
BSD 3-Clause "New" or "Revised" License
782 stars 126 forks source link

Fix two small issues that cause Windows problems #21

Closed borrrden closed 5 years ago

borrrden commented 5 years ago

This is also included in #17 but I figured I'd make it standalone as well since it is outside the context of TLS support.

fpagliughi commented 5 years ago

Yes. Cool. Better to consider some loosely-related fixes separately than in one big PR.

OMG, is the Windows SOCKET type unsigned?!?! If so, can the accept() call really be the only place in the code that this is a problem?

borrrden commented 5 years ago

Good thinking, there are probably more. I'll update the PR to fix the rest :p

borrrden commented 5 years ago

I counted roughly a dozen places that check_ret or check_ret_bool was used (all I could find). There was only one other function that returns SOCKET on Windows so I corrected it.

fpagliughi commented 5 years ago

Cool. I'll merge this shortly and take a look. I also grep'ed for any additional "< 0" comparisons, but I think we're close. I may have been aware of this when I first wrote the library, but forgotten it since then! There are a lot of explicit comparisons to the invalid socket constant, which I don't normally do in Linux programming. I used Windows a lot back then, but not really at all any more.

fpagliughi commented 5 years ago

I made one minor change... As check_socket() and check_socket_bool() are specifically meant to take sockets, I changed them from template functions to regular inline functions taking a socket_t value.

Pushed into develop.