Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strange build failure on i386 #93

Open
Peter2121 opened this issue Dec 21, 2023 · 5 comments
Open

Strange build failure on i386 #93

Peter2121 opened this issue Dec 21, 2023 · 5 comments
Assignees
Labels
fix added A fix was added but has yet to be fully tested/verified

Comments

@Peter2121
Copy link

The build bot of FreeBSD packages indicates a strange error during i386 package build:

/wrkdirs/usr/ports/net/libsockpp/work/sockpp-0.8.2/examples/udp/udpechosvr.cpp:75:8: error: call to member function 'send_to' is ambiguous
                sock.send_to(buf, n, srcAddr);
                ~~~~~^~~~~~~
/wrkdirs/usr/ports/net/libsockpp/work/sockpp-0.8.2/examples/udp/udpechosvr.cpp:114:13: note: in instantiation of function template specialization 'run_echo<sockpp::datagram_socket_tmpl<sockpp::inet_address>>' requested here
        thread thr(run_echo<sockpp::udp_socket>, std::move(udpsock));
                   ^
/wrkdirs/usr/ports/net/libsockpp/work/sockpp-0.8.2/include/sockpp/datagram_socket.h:369:10: note: candidate function
        ssize_t send_to(const std::string& s, int flags, const ADDR& addr) {
                ^
/wrkdirs/usr/ports/net/libsockpp/work/sockpp-0.8.2/include/sockpp/datagram_socket.h:379:10: note: candidate function
        ssize_t send_to(const void* buf, size_t n, const ADDR& addr) {
                ^

The full log is available here

Update of the port to 0.8.3 is pending, but I'm not sure it fixes the problem.

@fpagliughi
Copy link
Owner

I doubt v0.8.3 would fix this. It just added build support for unit tests with Catch2 v3.x.

But also note that the v0.8.x line was just promoted to v1.0, so I do want to make sure it's as stable and portable as possible. I don't know that I have an i386 compiler installed anywhere anymore, but I can try a couple different versions of Clang to see what I can find.

@fpagliughi
Copy link
Owner

I tried it on a number of x64 systems and compilers, and everything built cleanly.

Then, I tried a 32-bit Beaglebone Green that I had sitting on my desk, which already had Clang 13 installed. Sure enough, that hit it...

[ 65%] Building CXX object examples/udp/CMakeFiles/udpechosvr.dir/udpechosvr.cpp.o
/home/fmp/data/projects/cpp/sockpp.git/examples/udp/udpechosvr.cpp:75:14: error: call to member function 'send_to' is ambiguous
        sock.send_to(buf, n, srcAddr);
        ~~~~~^~~~~~~
/home/fmp/data/projects/cpp/sockpp.git/examples/udp/udpechosvr.cpp:112:16: note: in instantiation of function template specialization 'run_echo<sockpp::datagram_socket_tmpl<sockpp::inet_address>>' requested here
    thread thr(run_echo<sockpp::udp_socket>, std::move(udpsock));
               ^
/home/fmp/data/projects/cpp/sockpp.git/include/sockpp/datagram_socket.h:265:13: note: candidate function
    ssize_t send_to(const std::string& s, int flags, const ADDR& addr) {
            ^
/home/fmp/data/projects/cpp/sockpp.git/include/sockpp/datagram_socket.h:275:13: note: candidate function
    ssize_t send_to(const void* buf, size_t n, const ADDR& addr) {
            ^
/home/fmp/data/projects/cpp/sockpp.git/examples/udp/udpechosvr.cpp:75:14: error: call to member function 'send_to' is ambiguous
        sock.send_to(buf, n, srcAddr);
        ~~~~~^~~~~~~
/home/fmp/data/projects/cpp/sockpp.git/examples/udp/udpechosvr.cpp:116:5: note: in instantiation of function template specialization 'run_echo<sockpp::datagram_socket_tmpl<sockpp::inet6_address>>' requested here
    run_echo(std::move(udp6sock));
    ^
/home/fmp/data/projects/cpp/sockpp.git/include/sockpp/datagram_socket.h:265:13: note: candidate function
    ssize_t send_to(const std::string& s, int flags, const ADDR& addr) {
            ^
/home/fmp/data/projects/cpp/sockpp.git/include/sockpp/datagram_socket.h:275:13: note: candidate function
    ssize_t send_to(const void* buf, size_t n, const ADDR& addr) {
            ^
2 errors generated.
make[2]: *** [examples/udp/CMakeFiles/udpechosvr.dir/build.make:63: examples/udp/CMakeFiles/udpechosvr.dir/udpechosvr.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:498: examples/udp/CMakeFiles/udpechosvr.dir/all] Error 2
make: *** [Makefile:130: all] Error 2

Looking at it a little closer, I agree, it was a poor choice of overloads in that a char* could either convert to a void* or a std::string. I was obviously aiming for the const void* buf, size_t n variant, but I do see why the compiler is confused.

For now, perhaps an explicit cast in the example app would fix the build error for these 32-bit compilers.

But I may reconsider the form of that string overload in v2 of the library.

@fpagliughi fpagliughi added the fix added A fix was added but has yet to be fully tested/verified label Dec 22, 2023
@fpagliughi fpagliughi self-assigned this Dec 22, 2023
@fpagliughi
Copy link
Owner

fpagliughi commented Dec 22, 2023

I added a fix to the v1.x branch. If you test it and it works for you, I can release it as v0.1.0.1

Or if you need an actual release to put into the build pipeline for testing, I can push it out...

@fpagliughi
Copy link
Owner

fpagliughi commented Dec 22, 2023

D'Oh! 🤦‍♂️ Now I actually get it!

Say we boiled it down a little further to two overloads:

int send(const std::string& s, int flags);
int send(const void* buf, size_t n);

And say we try to call it like this:

const char* HELLO = "Hello there";
send(HELLO, strlen(HELLO));

The char* can be converted to either a void* or a string, so look to the 2nd parameter for a hint. On most 64-bit systems an int is still just 32-bits wide, whereas the size_t is 64-bits. So it matched strlen() to the size_t and chooses the 2nd variation.

But on 32-bit systems, both the int and the size_t are 32-bits! strlen() matches either equally.

So the call is ambiguous.
My bad.

I think the proper long-run solution is to get rid of the socket::send_to(const string&, int flags, ...) variation(s) from the library.

@Peter2121
Copy link
Author

It looks good now, no build errors reported with the patch integrated.
Please, create a new release containing the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix added A fix was added but has yet to be fully tested/verified
Projects
None yet
Development

No branches or pull requests

2 participants