Opened 4 years ago

Closed 4 years ago

#12825 closed bug (fixed)

[Patch] net_buffer.cpp: Fix clang warning

Reported by: mt Owned by: axeld
Priority: normal Milestone: Unscheduled
Component: Network & Internet/Stack Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

In remove_header(), 'while (left >= 0)' at line 1823 is always true because 'left' is unsigned. so I think we may change condition to 'while (left > 0)'.

/home/haiku/haiku/haiku/src/add-ons/kernel/network/stack/net_buffer.cpp:1823:14: warning: comparison of unsigned expression >= 0 is always true [-Wtautological-compare]
        while (left >= 0) {
               ~~~~ ^  ~

Attachments (1)

0012-net_buffer.cpp-fix-clang-warning.patch (892 bytes ) - added by mt 4 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 by mt, 4 years ago

Has a Patch: set

comment:2 by pulkomandy, 4 years ago

I'm not sure this is right, especially as the code inside the loop does handle "left == 0". Either we should make "left" signed, or change the loop into a "while true" if there are other ways to exit it.

comment:3 by TwoFx, 4 years ago

From my understanding of the code, the only way the loop ought to be exited is through the break statement in line 1827 and the return statement in line 1829. Therefore, putting while (true) should be appropriate. On the other hand, one might argue that while (left >= 0), despite doing nothing in practice, more clearly demonstrates the purpose of the loop (i.e. removing all bytes that are left).

comment:4 by axeld, 4 years ago

I'd prefer to use a while (true) loop here; especially since leaving it like this produces a warning, and is also slightly confusing for those who expect loop conditions make sense, and don't just hint at the intended usage :-)

In any case, the suggested patch is not correct, and would break the code, nice catch, Adrien!

comment:5 by pulkomandy, 4 years ago

Has a Patch: unset

comment:6 by axeld, 4 years ago

Status: newin-progress

comment:7 by axeld, 4 years ago

Resolution: fixed
Status: in-progressclosed

Fixed in hrev50429. Thanks mt for reporting!

Note: See TracTickets for help on using tickets.