test: fix sign comparison warning in socket tests #21159

fanquake commented at 2:15 AM on February 12, 2021: member

This fixes:

In file included from test/sock_tests.cpp:10:
In file included from /usr/local/include/boost/test/unit_test.hpp:18:
In file included from /usr/local/include/boost/test/test_tools.hpp:46:
/usr/local/include/boost/test/tools/old/impl.hpp:107:17: warning: comparison of integers of different signs: 'const long' and 'const unsigned long' [-Wsign-compare]
    return left == right;
           ~~~~ ^  ~~~~~

which was introduced in #20788.

test: fix sign comparison warning in socket tests

This fixes:
```bash
In file included from test/sock_tests.cpp:10:
In file included from /usr/local/include/boost/test/unit_test.hpp:18:
In file included from /usr/local/include/boost/test/test_tools.hpp:46:
/usr/local/include/boost/test/tools/old/impl.hpp:107:17: warning: comparison of integers of different signs: 'const long' and 'const unsigned long' [-Wsign-compare]
    return left == right;
           ~~~~ ^  ~~~~~
```

which was introduced in #20788.

9cc8e30125

fanquake added the label Tests on Feb 12, 2021

fanquake requested review from vasild on Feb 12, 2021

in src/test/sock_tests.cpp:98 in 9cc8e30125

  94 | @@ -95,7 +95,7 @@ static void CreateSocketPair(int s[2])
  95 |  static void SendAndRecvMessage(const Sock& sender, const Sock& receiver)
  96 |  {
  97 |      const char* msg = "abcd";
  98 | -    constexpr size_t msg_len = 4;
  99 | +    constexpr ssize_t msg_len = 4;

vasild commented at 8:25 AM on February 12, 2021:

This is used also as 3rd argument to strncmp() which is of type size_t. I hope this change will not produce a warning for that. If it does, then a typecast would be necessary.

vasild approved

vasild commented at 8:33 AM on February 12, 2021: member

ACK 9cc8e30125df14fe47e21e55ab3bf26f4d416565

Which compiler is that? I guess we don't have a CI run with that compiler and -Werror? Otherwise it would have been spotted before merge. I don't get a warning with or without this patch with Clang 12.

practicalswift commented at 9:49 AM on February 12, 2021: contributor

cr ACK 9cc8e30125df14fe47e21e55ab3bf26f4d416565

Agree with @vasild regarding adding -Werror for this diagnostic if possible :)

Sjors commented at 3:19 PM on February 12, 2021: member

Odd, I'm pretty we did have a -Werror check on Travis, back in the day...

Anyway, it fixes the error for me on macOS 11.2 with Boost 1.75

vasild commented at 3:35 PM on February 12, 2021: member

@Sjors, (without looking) I am sure there are some -Werror runs on CI, but probably not with the compiler that gets upset by this code.

If you observe the warning, which compiler is that?

No warning for me with Clang 12.0.0 and Boost 1.72.

Sjors commented at 4:08 PM on February 12, 2021: member

Apple clang version 12.0.0 (clang-1200.0.32.29) Target: x86_64-apple-darwin20.3.0

So perhaps the difference is in the boost version?

jonatack commented at 4:29 PM on February 12, 2021: member

I didn't see any warnings with Clang 9.0.1-16 and Boost 1.74.0.3 on Debian.

fanquake commented at 12:25 AM on February 17, 2021: member

It's unclear to me why the macOS 10.15 native CI run from #20788 didn't fail, as that should be the same setup of Apple Clang && boost-1.75.0_1 as where I'm seeing the warning. It was also being compiled with -Werror=sign-compare.

I don't think this is a Boost version difference, as I see the same warning whether I compile with Boost 1.71.0 out of depends:

In file included from test/sock_tests.cpp:10:
In file included from /Users/michael/github/fanquake-bitcoin/depends/x86_64-apple-darwin19.6.0/include/boost/test/unit_test.hpp:18:
In file included from /Users/michael/github/fanquake-bitcoin/depends/x86_64-apple-darwin19.6.0/include/boost/test/test_tools.hpp:46:
/Users/michael/github/fanquake-bitcoin/depends/x86_64-apple-darwin19.6.0/include/boost/test/tools/old/impl.hpp:107:17: warning: comparison of integers of different signs: 'const long' and 'const unsigned long' [-Wsign-compare]
    return left == right;
           ~~~~ ^  ~~~~~
/Users/michael/github/fanquake-bitcoin/depends/x86_64-apple-darwin19.6.0/include/boost/test/tools/old/impl.hpp:130:16: note: in instantiation of function template specialization 'boost::test_tools::tt_detail::equal_impl<long, unsigned long>' requested here
        return equal_impl( left, right );
               ^
/Users/michael/github/fanquake-bitcoin/depends/x86_64-apple-darwin19.6.0/include/boost/test/tools/old/impl.hpp:145:16: note: in instantiation of function template specialization 'boost::test_tools::tt_detail::equal_impl_frwd::call_impl<long, unsigned long>' requested here
        return call_impl( left, right, left_is_array() );
               ^
/Users/michael/github/fanquake-bitcoin/depends/x86_64-apple-darwin19.6.0/include/boost/test/tools/old/impl.hpp:92:50: note: in instantiation of function template specialization 'boost::test_tools::tt_detail::equal_impl_frwd::operator()<long, unsigned long>' requested here
BOOST_PP_REPEAT( BOOST_TEST_MAX_PREDICATE_ARITY, IMPL_FRWD, _ )
                                                 ^
/Users/michael/github/fanquake-bitcoin/depends/x86_64-apple-darwin19.6.0/include/boost/preprocessor/repetition/repeat.hpp:29:26: note: expanded from macro 'BOOST_PP_REPEAT'
# define BOOST_PP_REPEAT BOOST_PP_CAT(BOOST_PP_REPEAT_, BOOST_PP_AUTO_REC(BOOST_PP_REPEAT_P, 4))
                         ^
/Users/michael/github/fanquake-bitcoin/depends/x86_64-apple-darwin19.6.0/include/boost/preprocessor/cat.hpp:22:32: note: expanded from macro 'BOOST_PP_CAT'
#    define BOOST_PP_CAT(a, b) BOOST_PP_CAT_I(a, b)
                               ^
/Users/michael/github/fanquake-bitcoin/depends/x86_64-apple-darwin19.6.0/include/boost/preprocessor/cat.hpp:29:34: note: expanded from macro 'BOOST_PP_CAT_I'
#    define BOOST_PP_CAT_I(a, b) a ## b
                                 ^
<scratch space>:22:1: note: expanded from here
BOOST_PP_REPEAT_1
^
test/sock_tests.cpp:101:5: note: in instantiation of function template specialization 'boost::test_tools::tt_detail::check_frwd<boost::test_tools::tt_detail::equal_impl_frwd, long, unsigned long>' requested here
    BOOST_CHECK_EQUAL(sender.Send(msg, msg_len, 0), msg_len);
    ^
/Users/michael/github/fanquake-bitcoin/depends/x86_64-apple-darwin19.6.0/include/boost/test/tools/old/interface.hpp:154:45: note: expanded from macro 'BOOST_CHECK_EQUAL'
#define BOOST_CHECK_EQUAL( L, R )           BOOST_TEST_TOOL_IMPL( 0, \
                                            ^
/Users/michael/github/fanquake-bitcoin/depends/x86_64-apple-darwin19.6.0/include/boost/test/tools/old/interface.hpp:67:47: note: expanded from macro 'BOOST_TEST_TOOL_IMPL'
    BOOST_PP_IF( frwd_type, report_assertion, check_frwd ) (                    \
                                              ^
1 warning generated.

or 1.75.0 from brew:

In file included from test/sock_tests.cpp:10:
In file included from /usr/local/include/boost/test/unit_test.hpp:18:
In file included from /usr/local/include/boost/test/test_tools.hpp:46:
/usr/local/include/boost/test/tools/old/impl.hpp:107:17: warning: comparison of integers of different signs: 'const long' and 'const unsigned long' [-Wsign-compare]
    return left == right;
           ~~~~ ^  ~~~~~
/usr/local/include/boost/test/tools/old/impl.hpp:130:16: note: in instantiation of function template specialization 'boost::test_tools::tt_detail::equal_impl<long, unsigned long>' requested here
        return equal_impl( left, right );
               ^
/usr/local/include/boost/test/tools/old/impl.hpp:145:16: note: in instantiation of function template specialization 'boost::test_tools::tt_detail::equal_impl_frwd::call_impl<long, unsigned long>' requested here
        return call_impl( left, right, left_is_array() );
               ^
/usr/local/include/boost/test/tools/old/impl.hpp:92:50: note: in instantiation of function template specialization 'boost::test_tools::tt_detail::equal_impl_frwd::operator()<long, unsigned long>' requested here
BOOST_PP_REPEAT( BOOST_TEST_MAX_PREDICATE_ARITY, IMPL_FRWD, _ )
                                                 ^
/usr/local/include/boost/preprocessor/repetition/repeat.hpp:30:26: note: expanded from macro 'BOOST_PP_REPEAT'
# define BOOST_PP_REPEAT BOOST_PP_CAT(BOOST_PP_REPEAT_, BOOST_PP_AUTO_REC(BOOST_PP_REPEAT_P, 4))
                         ^
/usr/local/include/boost/preprocessor/cat.hpp:22:32: note: expanded from macro 'BOOST_PP_CAT'
#    define BOOST_PP_CAT(a, b) BOOST_PP_CAT_I(a, b)
                               ^
/usr/local/include/boost/preprocessor/cat.hpp:29:34: note: expanded from macro 'BOOST_PP_CA  CXX      test/test_bitcoin-util_threadnames_tests.o
T_I'
#    define BOOST_PP_CAT_I(a, b) a ## b
                                 ^
<scratch space>:154:1: note: expanded from here
BOOST_PP_REPEAT_1
^
test/sock_tests.cpp:101:5: note: in instantiation of function template specialization 'boost::test_tools::tt_detail::check_frwd<boost::test_tools::tt_detail::equal_impl_frwd, long, unsigned long>' requested here
    BOOST_CHECK_EQUAL(sender.Send(msg, msg_len, 0), msg_len);
    ^
/usr/local/include/boost/test/tools/old/interface.hpp:154:45: note: expanded from macro 'BOOST_CHECK_EQUAL'
#define BOOST_CHECK_EQUAL( L, R )           BOOST_TEST_TOOL_IMPL( 0, \
                                            ^
/usr/local/include/boost/test/tools/old/interface.hpp:67:47: note: expanded from macro 'BOOST_TEST_TOOL_IMPL'
    BOOST_PP_IF( frwd_type, report_assertion, check_frwd ) (                    \
                                              ^
1 warning generated.

fanquake merged this on Feb 17, 2021

fanquake closed this on Feb 17, 2021

fanquake deleted the branch on Feb 17, 2021

sidhujag referenced this in commit 7f50b7edf3 on Feb 17, 2021

MarcoFalke commented at 9:26 AM on February 17, 2021: member

See #19123

DrahtBot locked this on Aug 16, 2022