netif: fix compilation warning in QueryDefaultGatewayImpl() #34093

pull vasild wants to merge 1 commits into bitcoin:master from vasild:fix_nlmsg_ok_compilation_fbsd15 changing 1 files +6 −1
  1. vasild commented at 5:45 pm on December 17, 2025: contributor 
    0src/common/netif.cpp:137:51: error: comparison of integers of different signs: 'int64_t' (aka 'long') and 'unsigned long' [-Werror,-Wsign-compare]
1  137 |         for (nlmsghdr* hdr = (nlmsghdr*)response; NLMSG_OK(hdr, recv_result); hdr = NLMSG_NEXT(hdr, recv_result)) {
2      |                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~
3/usr/include/netlink/netlink.h:220:31: note: expanded from macro 'NLMSG_OK'
4  220 | #define NLMSG_OK(_hdr, _len)            NL_ITEM_OK(_hdr, _len, NLMSG_HDRLEN, _NLMSG_LEN)
5      |                                         ^                ~~~~  ~~~~~~~~~~~~
6/usr/include/netlink/netlink.h:203:10: note: expanded from macro 'NL_ITEM_OK'
7  203 |         ((_len) >= _hlen && _LEN_M(_ptr) >= _hlen && _LEN_M(_ptr) <= (_len))
8      |           ~~~~  ^  ~~~~~
91 error generated.

    Happens on FreeBSD 15.0, with the default compiler (Clang 19).

    On FreeBSD 14, /usr/include/netlink/netlink.h contains:

    0 #define NLMSG_HDRLEN                    ((int)sizeof(struct nlmsghdr))

    On FreeBSD 15, /usr/include/netlink/netlink.h contains:

    0 #define NLMSG_HDRLEN                    (sizeof(struct nlmsghdr))
  2. DrahtBot commented at 5:45 pm on December 17, 2025: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34093.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK hebasto

    If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

  3. DrahtBot added the label CI failed on Dec 17, 2025
  4. DrahtBot commented at 7:36 pm on December 17, 2025: contributor

    🚧 At least one of the CI tasks failed. Task fuzzer,address,undefined,integer: https://github.com/bitcoin/bitcoin/actions/runs/20312066455/job/58346231710 LLM reason (✨ experimental): Compilation failed due to a -Werror sign-compare error in src/common/netif.cpp (NLMSG_OK usage).

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  5. netif: fix compilation warning in QueryDefaultGatewayImpl() 
    ```
src/common/netif.cpp:137:51: error: comparison of integers of different signs: 'int64_t' (aka 'long') and 'unsigned long' [-Werror,-Wsign-compare]
  137 |         for (nlmsghdr* hdr = (nlmsghdr*)response; NLMSG_OK(hdr, recv_result); hdr = NLMSG_NEXT(hdr, recv_result)) {
      |                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/netlink/netlink.h:220:31: note: expanded from macro 'NLMSG_OK'
  220 | #define NLMSG_OK(_hdr, _len)            NL_ITEM_OK(_hdr, _len, NLMSG_HDRLEN, _NLMSG_LEN)
      |                                         ^                ~~~~  ~~~~~~~~~~~~
/usr/include/netlink/netlink.h:203:10: note: expanded from macro 'NL_ITEM_OK'
  203 |         ((_len) >= _hlen && _LEN_M(_ptr) >= _hlen && _LEN_M(_ptr) <= (_len))
      |           ~~~~  ^  ~~~~~
1 error generated.
```

Happens on FreeBSD 15.0, with the default compiler (Clang 19).

On FreeBSD 14, `/usr/include/netlink/netlink.h` contains:
```
 #define NLMSG_HDRLEN                    ((int)sizeof(struct nlmsghdr))
```

On FreeBSD 15, `/usr/include/netlink/netlink.h` contains:
```
 #define NLMSG_HDRLEN                    (sizeof(struct nlmsghdr))
```
    be2a6248fb
  6. vasild force-pushed on Dec 18, 2025
  7. vasild commented at 11:27 am on December 18, 2025: contributor

    I am not sure why there is no “comparison of integers of different signs” on Linux:

    0#define NLMSG_OK(nlh,len) ((len) >= (int)sizeof(struct nlmsghdr) && \
1                           (nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \
2                           (nlh)->nlmsg_len <= (len))

    We pass int64_t for len. The first comparison checks it against (int)sizeof(..., but the 3rd comparison checks it against nlmsg_len which is __u32, so no matter what type is passed for len, one of the two comparisons is going to be of different sign, yet there is no warning :eyes: and if I pass unsigned type for len then I get a warning in the first comparison :fire:

  8. DrahtBot removed the label CI failed on Dec 18, 2025
  9. fanquake added the label Needs backport (30.x) on Dec 18, 2025
  10. maflcko commented at 3:35 pm on December 18, 2025: member

    I am not sure why there is no “comparison of integers of different signs” on Linux:

    0#define NLMSG_OK(nlh,len) ((len) >= (int)sizeof(struct nlmsghdr) && \
1                           (nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \
2                           (nlh)->nlmsg_len <= (len))

    We pass int64_t for len. The first comparison checks it against (int)sizeof(..., but the 3rd comparison checks it against nlmsg_len which is __u32, so no matter what type is passed for len, one of the two comparisons is going to be of different sign, yet there is no warning 👀 and if I pass unsigned type for len then I get a warning in the first comparison 🔥

    I guess (int)sizeof(struct nlmsghdr) is a compile-time constant, so the compiler can see the value, or at least can see that it is never negative, so there is no issue?

  11. hebasto approved
  12. hebasto commented at 7:10 pm on December 18, 2025: member

    ACK be2a6248fbc6ade0c0cd734245138f83f3202266. See the following CI jobs:

  13. vasild commented at 5:57 am on December 19, 2025: contributor

    this PR on FreeBSD 15.0: https://github.com/hebasto/bitcoin-core-nightly/actions/runs/20347897719

    The job is red but it compiles and then fails due to:

    feature_reindex_init.py | ✖ Failed | 1 s

    (offtopic) I have been annoyed by this failure, will try to look at it sometime soon.

    I guess (int)sizeof(struct nlmsghdr) is a compile-time constant, so the compiler can see the value, or at least can see that it is never negative, so there is no issue?

    Must be something more subtle because the following program:

     0#include <netlink/netlink.h>
 1#include <netlink/netlink_route.h>
 2
 3int main(int argc, char** argv)
 4{
 5    int64_t recv_result{static_cast<int64_t>(argc)};
 6
 7    int sizeof_struct_nlmsghdr{argc};
 8    uint32_t nlmsg_len{static_cast<uint32_t>(argc)};
 9
10    if (recv_result >= sizeof_struct_nlmsghdr) { 
11        return 0;
12    } 
13    if (nlmsg_len <= recv_result) { 
14        return 0;
15    } 
16    return 0;
17}

    does not emit a warning and none of the variables is a compile time constant. I would expect a warning for the second if which compares uint32_t and int64_t.

    I guess (int)sizeof(struct nlmsghdr) is a compile-time constant…

    That comparison is int64_t vs int - no warning should be emitted even if the compiler does not see that sizeof is compile time positive constant.

  14. maflcko commented at 6:06 am on December 19, 2025: member

    does not emit a warning and none of the variables is a compile time constant. I would expect a warning for the second if which compares uint32_t and int64_t.

    Why should it warn? uint32_t is of size 32, so it can perfectly fit into an int64_t and the compare can never behave unexpectedly?

  15. hebasto commented at 10:37 am on December 19, 2025: member

    this PR on FreeBSD 15.0: https://github.com/hebasto/bitcoin-core-nightly/actions/runs/20347897719

    The job is red but it compiles and then fails due to:

    feature_reindex_init.py | ✖ Failed | 1 s

    (offtopic) I have been annoyed by this failure, will try to look at it sometime soon.

    Maybe #33128?

  16. vasild commented at 11:52 am on December 19, 2025: contributor

    I found this useful on the topic of integers comparison: This https://www.cppstories.com/2022/safe-int-cmp-cpp20/:

    • If both operands are signed or both are unsigned, the operand with lesser conversion rank is converted to the operand with the greater integer conversion rank.
    • Otherwise, if the unsigned operand’s conversion rank is greater or equal to the conversion rank of the signed operand, the signed operand is converted to the unsigned operand’s type.
    • Otherwise, if the signed operand’s type can represent all values of the unsigned operand, the unsigned operand is converted to the signed operand’s type.
    • Otherwise, both operands are converted to the unsigned counterpart of the signed operand’s type.

    The conversion rank above increases in order bool, signed char, short, int, long, long long (since C++11). The rank of any unsigned type is equal to the rank of the corresponding signed type.

    but:

     0(lldb) p (int8_t)-1 < (uint8_t)1
 1(bool) true
 2
 3(lldb) p (int16_t)-1 < (uint16_t)1
 4(bool) true
 5
 6(lldb) p (int32_t)-1 < (uint32_t)1
 7(bool) false
 8
 9(lldb) p (int64_t)-1 < (uint64_t)1
10(bool) false

    :eyes:

  17. maflcko commented at 12:13 pm on December 19, 2025: member

    I found this useful on the topic of integers comparison: This https://www.cppstories.com/2022/safe-int-cmp-cpp20/:

    • If both operands are signed or both are unsigned, the operand with lesser conversion rank is converted to the operand with the greater integer conversion rank.
    • Otherwise, if the unsigned operand’s conversion rank is greater or equal to the conversion rank of the signed operand, the signed operand is converted to the unsigned operand’s type.
    • Otherwise, if the signed operand’s type can represent all values of the unsigned operand, the unsigned operand is converted to the signed operand’s type.
    • Otherwise, both operands are converted to the unsigned counterpart of the signed operand’s type.

    The conversion rank above increases in order bool, signed char, short, int, long, long long (since C++11). The rank of any unsigned type is equal to the rank of the corresponding signed type.

    but:

     0(lldb) p (int8_t)-1 < (uint8_t)1
 1(bool) true
 2
 3(lldb) p (int16_t)-1 < (uint16_t)1
 4(bool) true
 5
 6(lldb) p (int32_t)-1 < (uint32_t)1
 7(bool) false
 8
 9(lldb) p (int64_t)-1 < (uint64_t)1
10(bool) false

    👀

    I haven’t read the blog post, but generally, cpprefernce is a better source about C++. It says https://en.cppreference.com/w/cpp/language/implicit_cast.html#Integral_promotion:

    prvalues of small integral types (such as char) and unscoped enumeration types may be converted to prvalues of larger integral types (such as int).

    So I think everything is expected here? Also, I don’t think any of the NLMSG macros deal with 16-bit ints, or lower.

  18. fanquake requested review from laanwj on Dec 19, 2025
  19. hebasto referenced this in commit 48a7c98287 on Dec 27, 2025
  20. luke-jr referenced this in commit 032391cb6e on Jan 13, 2026
  21. hebasto commented at 5:05 pm on January 28, 2026: member

    Happens on FreeBSD 15.0, with the default compiler (Clang 19).

    It’s also happening on FreeBSD 14.3 now, with the default Clang 19.1.7.

    UPDATE: #34093 (comment).

  22. maflcko commented at 5:18 pm on January 28, 2026: member
    Should probably derive the type to use via decltype and possibly some C++ metaprogramming
  23. vasild commented at 3:33 pm on February 2, 2026: contributor

    Happens on FreeBSD 15.0, with the default compiler (Clang 19).

    It’s also happening on FreeBSD 14.3 now, with the default Clang 19.1.7.

    Hmm, the latest 14 still has NLMSG_HDRLEN as int (unchanged):

    https://cgit.freebsd.org/src/tree/sys/netlink/netlink.h?h=stable/14#n216

    !?

  24. hebasto commented at 8:52 pm on February 2, 2026: member

    Happens on FreeBSD 15.0, with the default compiler (Clang 19).

    It’s also happening on FreeBSD 14.3 now, with the default Clang 19.1.7.

    Hmm, the latest 14 still has NLMSG_HDRLEN as int (unchanged):

    https://cgit.freebsd.org/src/tree/sys/netlink/netlink.h?h=stable/14#n216

    !?

    Upgraded compiler?

  25. vasild commented at 7:42 am on February 3, 2026: contributor

    Shouldn’t be compiler dependent. The original problem, currently in master is that it is comparing int64_t vs size_t on FreeBSD 15. On FreeBSD 14 it is ok because it is comparing int64_t vs int, based on netlink.h (not based on compiler version).

    What is the full error that you observe on FreeBSD 14?

  26. hebasto commented at 1:38 pm on February 3, 2026: member

    What is the full error that you observe on FreeBSD 14?

     0[98/835] Building CXX object src/CMakeFiles/bitcoin_common.dir/common/netif.cpp.o
 1/home/runner/work/bitcoin-core-nightly/bitcoin-core-nightly/src/common/netif.cpp:137:51: warning: comparison of integers of different signs: 'int64_t' (aka 'long') and 'unsigned long' [-Wsign-compare]
 2  137 |         for (nlmsghdr* hdr = (nlmsghdr*)response; NLMSG_OK(hdr, recv_result); hdr = NLMSG_NEXT(hdr, recv_result)) {
 3      |                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~
 4/usr/include/netlink/netlink.h:220:31: note: expanded from macro 'NLMSG_OK'
 5  220 | #define NLMSG_OK(_hdr, _len)            NL_ITEM_OK(_hdr, _len, NLMSG_HDRLEN, _NLMSG_LEN)
 6      |                                         ^                ~~~~  ~~~~~~~~~~~~
 7/usr/include/netlink/netlink.h:203:10: note: expanded from macro 'NL_ITEM_OK'
 8  203 |         ((_len) >= _hlen && _LEN_M(_ptr) >= _hlen && _LEN_M(_ptr) <= (_len))
 9      |           ~~~~  ^  ~~~~~
101 warning generated.

    More logs are available here: https://github.com/hebasto/bitcoin-core-nightly/actions/runs/21632105860.

    UPDATE. Disregard this comment as it seems the GHA workflow I mentioned is broken.

  27. hebasto commented at 2:25 pm on February 3, 2026: member

    Happens on FreeBSD 15.0, with the default compiler (Clang 19).

    It’s also happening on FreeBSD 14.3 now, with the default Clang 19.1.7.

    This comment is incorrect. I used a log from the broken GHA workflow. My apologies.


vasild DrahtBot maflcko hebasto


laanwj

Labels
Needs backport (30.x)

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-02-18 12:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me