search

naemon / naemon-livestatus

Naemon - Livestatus Eventbroker Module
GNU General Public License v2.0
26 stars 30 forks source link

fix compiler warnings on EL6,EL7,EL8 #76

Open pbiering opened 4 years ago

pbiering commented 4 years ago

split-out of https://github.com/naemon/naemon-livestatus/pull/71

pbiering commented 4 years ago

Overall looks good, but could you have a look at all the changes from unsigned to signed. I think all cases it makes sense to have unsigned, as negative numbers makes no sense in that context.

in underlying resulting assembler code, usually comparison of integers are done by substracting them (and therefore converting them to signed integer before) and check whether result is greater/equal/lower than zero. Looks like compilers get more strict here to prevent any accidents by substracting integers which have value >= 2^(bitlength-1), because then the bit for the +/- get lost.

You can avoid the compiler warning by casting during comparison, but this is also not a nice way and can cause issues for same reason.

jacobbaungard commented 4 years ago

What I am saying is: We should use consistent types throughout the source, and it makes no sense to change an value which is intrinsically non-negative to a signed integer, just because somewhere we are doing a comparison with a signed integer. Instead we should ensure the other variable we are comparing with is also unsigned.

For example rather than changing _max_cached_messages to signed, we should change num_cached_log_messages to unsigned.

pbiering commented 4 years ago

What I am saying is: We should use consistent types throughout the source, and it makes no sense to change an value which is intrinsically non-negative to a signed integer, just because somewhere we are doing a comparison with a signed integer. Instead we should ensure the other variable we are comparing with is also unsigned.

For example rather than changing _max_cached_messages to signed, we should change num_cached_log_messages to unsigned.

feel free to fix the other side...I have only tried to fix where compiler warned me -> cherry pick the other valid changes and adjust the others...