Repository navigation
curl_stream: use CURLINFO_ACTIVESOCKET if curl is recent enough - #3996
carlosmn merged 1 commit into
Conversation
Sorry, something went wrong.
|
While I don't like the conditional compile, there's no real alternative I could think of. We definitly cannot drop support for libcurl<7.45.0 but we also have to fix the issue if possible. I also wanted to write a test for #3993, but we actually have no abstractions around socket, listen and accept and I didn't want to introduce them just for the sake of testing. We might as well just compile the test on POSIX-systems, but yeah... |
Sorry, something went wrong.
|
I'd rather check for the flag existing in the headers rather than the version of the library. I would also much prefer for us to create a #define for the curl type if the library version doesn't define it so we can write the code against as though we had the newer version available, and the defines take care of using the right value. |
Sorry, something went wrong.
The `CURLINFO_LASTSOCKET` information has been deprecated since curl version 7.45.0 as it may result in an overflow in the returned socket on certain systems, most importantly on 64 bit Windows. Instead, a new call `CURLINFO_ACTIVESOCKET` has been added which instead returns a `curl_socket_t`, which is always sufficiently long to store a socket. As we need to provide backwards compatibility with curl versions smaller than 7.45.0, alias CURLINFO_ACTIVESOCKET to CURLINFO_LASTSOCKET on platforms without CURLINFO_ACTIVESOCKET.
|
Well, this is how it looks like when #define'ing things instead. Probably looks a bit nicer. Would be nice if we could simply define things like "CURL_SOCKET_BAD" instead of introducing "GIT_CURL_BADSOCKET", but unfortunately these macros existed earlier than 7.45.0 and have wrong semantics on Windows platforms in that they work with Window's SOCKET type. |
Sorry, something went wrong.
|
Yeah, I do like this better. The version-dependent changes are all together in a single block rather than mixed together with real code. |
Sorry, something went wrong.
The CURLINFO_LASTSOCKET information has been deprecated since
curl version 7.45.0 as it may result in an overflow in the
returned socket on certain systems, most importantly on 64 bit
Windows. Instead, a new call CURLINFO_ACTIVESOCKET has been
added which instead returns a curl_socket_t, which is always
sufficiently long to store a socket.
As the new call has only been introduced with version 7.45.0
which was released in October 2015, we cannot simply remove the
old code but instead are required to conditionally compile based
on libcurl's version.