Fix SelectInterruptPipe::read() being called with fd -1

[cosminpolifronie]

When using IXWebSocket in our enterprise project, we are getting the following valgrind warning.
Pull request #503 is handling this issue by adding a check to the read location.
This pull requests fixes the root cause, where a SelectInterruptPtr is used without being initialized. This SelectInterruptPtr is not needed at all at this moment in the code path, so it has been completely replaced with a nullptr.

Build and tests are passing, and also our enterprise project is not showing the valgrind warning anymore.

Also, to answer #503 (comment), the correct error return value at this place is 0, as the parent class SelectInterrupt also returns 0 (signaling an error).
https://github.com/machinezone/IXWebSocket/blob/master/ixwebsocket/IXSelectInterrupt.cpp#L36

==7952== Warning: invalid file descriptor -1 in syscall read()
==7952==    at 0x4D17ACA: __libc_read (read.c:26)
==7952==    by 0x4D17ACA: read (read.c:24)
==7952==    by 0x16DFCCE: ix::SelectInterruptPipe::read() (IXSelectInterruptPipe.cpp:146)
==7952==    by 0x16DE646: readSelectInterruptRequest (IXSocket.cpp:161)
==7952==    by 0x16DE646: ix::Socket::poll(bool, int, int, std::unique_ptr<ix::SelectInterrupt, std::default_delete<ix::SelectInterrupt> > const&) (IXSocket.cpp:89)
==7952==    by 0x16DFF0E: ix::SocketConnect::connectToAddress(addrinfo const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::function<bool ()> const&) (IXSocketConnect.cpp:71)
==7952==    by 0x16E030D: ix::SocketConnect::connect(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::function<bool ()> const&) (IXSocketConnect.cpp:120)
==7952==    by 0x16DE946: ix::Socket::connect(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::function<bool ()> const&) (IXSocket.cpp:229)
==7952==    by 0x16E2277: ix::WebSocketHandshake::clientHandshake(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, ix::CaseInsensitiveLess, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int, int) (IXWebSocketHandshake.cpp:102)
==7952==    by 0x16D7559: ix::WebSocketTransport::connectToUrl(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, ix::CaseInsensitiveLess, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > const&, int) (IXWebSocketTransport.cpp:142)
==7952==    by 0x16D216F: ix::WebSocket::connect(int) (IXWebSocket.cpp:233)
==7952==    by 0x16D2B02: ix::WebSocket::checkConnection(bool) (IXWebSocket.cpp:342)
==7952==    by 0x16D1C6F: ix::WebSocket::run() (IXWebSocket.cpp:384)
==7952==    by 0x4A3ADB3: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.33)

[bsergean]

I think it's because we don't call init() on that object.

IXWebSocket/ixwebsocket/IXSelectInterruptPipe.cpp

Line 47 in a0e45c4

bool SelectInterruptPipe::init(std::string& errorMsg)

Can you try that, this should become a one liner fix.

[cosminpolifronie]

Would this be needed at this stage? As far as I understand from the code the SelectInterrupt is never used again, it was there just to be able to call poll().

[cosminpolifronie]

Copilot also cannot find any use for a SelectInterrupt at that location, and also suggest a nullptr.

[bsergean]

What's wrong with ret for a variable name ?

[cosminpolifronie]

Nothing inherently wrong, just gave me the impression that it will be used as the return value for the whole function at first. Just makes the code a bit faster to read for me.

[bsergean]

Ok. In general it's better to keep PR to the minimum viable change, this is just marginally better and debatable, it's about taste I like short names better as long as they aren't cryptic like x.

[cosminpolifronie]

I can see the builds were started before #571 was merged, which contained some mbedtls build system fixes. Could you please re-run the builds so I can focus on the actual issues introduced by this PR?

[cosminpolifronie]

Builds are passing now. It seems mbedtls4 was still used on some build paths, and I also had to merge master to my branch for commit 5ada5f7 to be used in the build.

The uwp failed build has run into trouble trying to download ninja-build. It will probably run fine on a re-run.