bug list: assigning negative values to unsigned variables
Fixing the places which assign negative values to unsigned variables is a good janitor task.
This list is from smatch: grep -w "fit into 0" warns.txt | grep -v "\-1" | tee err-list
regards,
dan carpenter
drivers/bluetooth/btmrvl_main.c +74 btmrvl_process_event(9) warn: value -22 can't fit into 0 'ret' drivers/bluetooth/btmrvl_main.c +123 btmrvl_process_event(58) warn: value -22 can't fit into 0 'ret' drivers/bluetooth/btmrvl_main.c +136 btmrvl_process_event(71) warn: value -22 can't fit into 0 'ret' drivers/char/mwave/smapi.c +69 smapi_request(12) warn: value -5 can't fit into 0 'usSmapiOK' drivers/net/ixgbe/ixgbe_dcb_nl.c +439 ixgbe_dcbnl_getcap(29) warn: value -22 can't fit into 0 'rval' drivers/net/ixgbe/ixgbe_dcb_nl.c +443 ixgbe_dcbnl_getcap(33) warn: value -22 can't fit into 0 'rval' drivers/net/ixgbe/ixgbe_dcb_nl.c +463 ixgbe_dcbnl_getnumtcs(14) warn: value -22 can't fit into 0 'rval' drivers/net/ixgbe/ixgbe_dcb_nl.c +467 ixgbe_dcbnl_getnumtcs(18) warn: value -22 can't fit into 0 'rval' drivers/net/wireless/b43legacy/phy.c +1205 b43legacy_phy_lo_b_measure(60) warn: value -772 can't fit into 0 'fval' drivers/net/wireless/b43legacy/phy.c +1632 b43legacy_phy_lo_g_measure(143) warn: value -3 can't fit into 0 'tmp' drivers/net/wireless/hostap/hostap_ioctl.c +1020 prism2_ioctl_giwrange(66) warn: value -60 can't fit into 0 'range->avg_qual.level' drivers/net/wireless/hostap/hostap_ioctl.c +1021 prism2_ioctl_giwrange(67) warn: value -95 can't fit into 0 'range->avg_qual.noise' drivers/net/wireless/ipw2x00/ipw2100.c +6901 ipw2100_wx_get_range(37) warn: value -78 can't fit into 0 'range->avg_qual.level' drivers/net/wireless/libertas/wext.c +875 lbs_get_wireless_stats(34) warn: value -96 can't fit into 0 'priv->wstats.qual.noise' drivers/net/wireless/prism54/isl_ioctl.c +452 prism54_get_range(33) warn: value -80 can't fit into 0 'range->avg_qual.level' drivers/net/bnx2x_link.c +3189 bnx2x_sfp_module_detection(21) warn: value -22 can't fit into 0 'rc' drivers/net/bnx2x_link.c +4548 bnx2x_ext_phy_init(911) warn: value -22 can't fit into 0 'rc' drivers/net/bnx2x_link.c +4553 bnx2x_ext_phy_init(916) warn: value -22 can't fit into 0 'rc' drivers/net/bnx2x_link.c +5556 bnx2x_get_ext_phy_fw_version(53) warn: value -22 can't fit into 0 'status' drivers/net/bnx2x_link.c +5854 bnx2x_set_led(59) warn: value -22 can't fit into 0 'rc' drivers/staging/rt2860/sta_ioctl.c +564 rt_ioctl_giwrange(60) warn: value -60 can't fit into 0 'range->avg_qual.level' drivers/staging/rt2860/sta_ioctl.c +565 rt_ioctl_giwrange(61) warn: value -95 can't fit into 0 'range->avg_qual.noise' drivers/staging/rt2870/../rt2860/sta_ioctl.c +564 rt_ioctl_giwrange(60) warn: value -60 can't fit into 0 'range->avg_qual.level' drivers/staging/rt2870/../rt2860/sta_ioctl.c +565 rt_ioctl_giwrange(61) warn: value -95 can't fit into 0 'range->avg_qual.noise' drivers/staging/rtl8187se/r8180_wx.c +275 rtl8180_wx_get_range(39) warn: value -98 can't fit into 0 'range->max_qual.noise' drivers/staging/rtl8187se/r8180_wx.c +280 rtl8180_wx_get_range(44) warn: value -78 can't fit into 0 'range->avg_qual.level' drivers/staging/rtl8192e/r8192E_wx.c +445 rtl8180_wx_get_range(39) warn: value -98 can't fit into 0 'range->max_qual.noise' drivers/staging/rtl8192e/r8192E_wx.c +450 rtl8180_wx_get_range(44) warn: value -78 can't fit into 0 'range->avg_qual.level' drivers/staging/rtl8192su/r8192U_wx.c +434 rtl8180_wx_get_range(39) warn: value -98 can't fit into 0 'range->max_qual.noise' drivers/staging/rtl8192su/r8192U_wx.c +439 rtl8180_wx_get_range(44) warn: value -78 can't fit into 0 'range->avg_qual.level' drivers/staging/rtl8192u/r8192U_wx.c +420 rtl8180_wx_get_range(39) warn: value -98 can't fit into 0 'range->max_qual.noise' drivers/staging/rtl8192u/r8192U_wx.c +425 rtl8180_wx_get_range(44) warn: value -78 can't fit into 0 'range->avg_qual.level' drivers/usb/serial/iuu_phoenix.c +565 iuu_clk(125) warn: value -2 can't fit into 0 '*(priv->buf+Count++)'
Quoted 1 line · Dan Carpenter
Fixing the places which assign negative values to unsigned variables is a good janitor task.
I had the impression that assignment to -1 was done sometimes as a portable way to initialize the variable to 0xffff (for any number of f's). So perhaps it is not so trivial to fix.
julia
Quoted 43 lines
This list is from smatch: grep -w "fit into 0" warns.txt | grep -v "\-1" | tee err-list
regards,
dan carpenter
drivers/bluetooth/btmrvl_main.c+74btmrvl_process_event(9)warn: value -22 can't fit into 0 'ret'drivers/bluetooth/btmrvl_main.c+123btmrvl_process_event(58)warn: value -22 can't fit into 0 'ret'drivers/bluetooth/btmrvl_main.c+136btmrvl_process_event(71)warn: value -22 can't fit into 0 'ret'drivers/char/mwave/smapi.c+69smapi_request(12)warn: value -5 can't fit into 0 'usSmapiOK'drivers/net/ixgbe/ixgbe_dcb_nl.c+439ixgbe_dcbnl_getcap(29)warn: value -22 can't fit into 0 'rval'drivers/net/ixgbe/ixgbe_dcb_nl.c+443ixgbe_dcbnl_getcap(33)warn: value -22 can't fit into 0 'rval'drivers/net/ixgbe/ixgbe_dcb_nl.c+463ixgbe_dcbnl_getnumtcs(14)warn: value -22 can't fit into 0 'rval'drivers/net/ixgbe/ixgbe_dcb_nl.c+467ixgbe_dcbnl_getnumtcs(18)warn: value -22 can't fit into 0 'rval'drivers/net/wireless/b43legacy/phy.c+1205b43legacy_phy_lo_b_measure(60)warn: value -772 can't fit into 0 'fval'drivers/net/wireless/b43legacy/phy.c+1632b43legacy_phy_lo_g_measure(143)warn: value -3 can't fit into 0 'tmp'drivers/net/wireless/hostap/hostap_ioctl.c+1020prism2_ioctl_giwrange(66)warn: value -60 can't fit into 0 'range->avg_qual.level'drivers/net/wireless/hostap/hostap_ioctl.c+1021prism2_ioctl_giwrange(67)warn: value -95 can't fit into 0 'range->avg_qual.noise'drivers/net/wireless/ipw2x00/ipw2100.c+6901ipw2100_wx_get_range(37)warn: value -78 can't fit into 0 'range->avg_qual.level'drivers/net/wireless/libertas/wext.c+875lbs_get_wireless_stats(34)warn: value -96 can't fit into 0 'priv->wstats.qual.noise'drivers/net/wireless/prism54/isl_ioctl.c+452prism54_get_range(33)warn: value -80 can't fit into 0 'range->avg_qual.level'drivers/net/bnx2x_link.c+3189bnx2x_sfp_module_detection(21)warn: value -22 can't fit into 0 'rc'drivers/net/bnx2x_link.c+4548bnx2x_ext_phy_init(911)warn: value -22 can't fit into 0 'rc'drivers/net/bnx2x_link.c+4553bnx2x_ext_phy_init(916)warn: value -22 can't fit into 0 'rc'drivers/net/bnx2x_link.c+5556bnx2x_get_ext_phy_fw_version(53)warn: value -22 can't fit into 0 'status'drivers/net/bnx2x_link.c+5854bnx2x_set_led(59)warn: value -22 can't fit into 0 'rc'drivers/staging/rt2860/sta_ioctl.c+564rt_ioctl_giwrange(60)warn: value -60 can't fit into 0 'range->avg_qual.level'drivers/staging/rt2860/sta_ioctl.c+565rt_ioctl_giwrange(61)warn: value -95 can't fit into 0 'range->avg_qual.noise'drivers/staging/rt2870/../rt2860/sta_ioctl.c+564rt_ioctl_giwrange(60)warn: value -60 can't fit into 0 'range->avg_qual.level'drivers/staging/rt2870/../rt2860/sta_ioctl.c+565rt_ioctl_giwrange(61)warn: value -95 can't fit into 0 'range->avg_qual.noise'drivers/staging/rtl8187se/r8180_wx.c+275rtl8180_wx_get_range(39)warn: value -98 can't fit into 0 'range->max_qual.noise'drivers/staging/rtl8187se/r8180_wx.c+280rtl8180_wx_get_range(44)warn: value -78 can't fit into 0 'range->avg_qual.level'drivers/staging/rtl8192e/r8192E_wx.c+445rtl8180_wx_get_range(39)warn: value -98 can't fit into 0 'range->max_qual.noise'drivers/staging/rtl8192e/r8192E_wx.c+450rtl8180_wx_get_range(44)warn: value -78 can't fit into 0 'range->avg_qual.level'drivers/staging/rtl8192su/r8192U_wx.c+434rtl8180_wx_get_range(39)warn: value -98 can't fit into 0 'range->max_qual.noise'drivers/staging/rtl8192su/r8192U_wx.c+439rtl8180_wx_get_range(44)warn: value -78 can't fit into 0 'range->avg_qual.level'drivers/staging/rtl8192u/r8192U_wx.c+420rtl8180_wx_get_range(39)warn: value -98 can't fit into 0 'range->max_qual.noise'drivers/staging/rtl8192u/r8192U_wx.c+425rtl8180_wx_get_range(44)warn: value -78 can't fit into 0 'range->avg_qual.level'drivers/usb/serial/iuu_phoenix.c+565iuu_clk(125)warn: value -2 can't fit into 0 '*(priv->buf+Count++)'
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 7 lines · Julia Lawall
Quoted 1 line · Dan Carpenter
Fixing the places which assign negative values to unsigned variables is a good janitor task.
I had the impression that assignment to -1 was done sometimes as a portable way to initialize the variable to
0xffff(for any number of f's). So perhaps it is not so trivial to fix.
Any particular reason that ~0U, ~0UL, and ~0ULL shouldn't do the same (without relying on conversion from signed to unsigned)?
Bernd
Signature · 2 lines
Bernd Petrovitsch Email : [email protected] LUGA : http://www.luga.at
Quoted 10 lines · Bernd Petrovitsch
Quoted 7 lines · Julia Lawall
Quoted 1 line · Dan Carpenter
Fixing the places which assign negative values to unsigned variables is a good janitor task.
I had the impression that assignment to -1 was done sometimes as a portable way to initialize the variable to
0xffff(for any number of f's). So perhaps it is not so trivial to fix.Any particular reason that ~0U, ~0UL, and ~0ULL shouldn't do the same (without relying on conversion from signed to unsigned)?
Then the constant specifies the type?
julia
Quoted 8 lines · Julia Lawall
Quoted 7 lines · Bernd Petrovitsch
Quoted 6 lines · Julia Lawall
Quoted 1 line · Dan Carpenter
Fixing the places which assign negative values to unsigned variables is a good janitor task.
I had the impression that assignment to -1 was done sometimes as a portable way to initialize the variable to
0xffff(for any number of f's).
Hmm, perhaps some experienced language lawyer can comment on the "portable".
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
So perhaps it is not so trivial to fix.
Any particular reason that ~0U, ~0UL, and ~0ULL shouldn't do the same (without relying on conversion from signed to unsigned)?
Then the constant specifies the type?
Yes. And it is necessary as "~0U" assigned to a "unsigned long long int" won't give "~0ULL".
Otherwise "0" would be a signed int and from then on (starting with "~0") we are in the C hell of type promotion/conversion from signed to unsigned and/or back - at least in theory.
Bernd
Signature · 2 lines
Bernd Petrovitsch Email : [email protected] LUGA : http://www.luga.at
Quoted 11 lines · Bernd Petrovitsch
Quoted 8 lines · Julia Lawall
Quoted 7 lines · Bernd Petrovitsch
Quoted 6 lines · Julia Lawall
Quoted 1 line · Dan Carpenter
Fixing the places which assign negative values to unsigned variables is a good janitor task.
I had the impression that assignment to -1 was done sometimes as a portable way to initialize the variable to
0xffff(for any number of f's).Hmm, perhaps some experienced language lawyer can comment on the "portable".
Doesn't take a lawyer; conversion to unsigned types *IS* portable and defined as arithmetics modulo 2^{width}. In particular, for any unsigned type T you are going to have the same results from (T)-1 and ~(T)0 (and (T)-1L, etc.). The value being converted is interpreted as an integer (i.e. the element of $\Bbb Z$) and then taken modulo 2^{width}, regardless of the type it had come from. So -1 is just fine and will result in 0xff....f of the right width.
It's conversion to signed that is a mess if the value you are converting isn't already in range representable by the type you are converting to.
Whether a specific example of conversion of negative to unsigned is a good idea stylistically is a different question, of course, but that should be taken on case-to-case basis.
Quoted 10 lines · Julia Lawall
Quoted 1 line · Dan Carpenter
Fixing the places which assign negative values to unsigned variables is a good janitor task.
I had the impression that assignment to -1 was done sometimes as a portable way to initialize the variable to
0xffff(for any number of f's). So perhaps it is not so trivial to fix.julia
The grep removed those.
Quoted 1 line
Quoted 1 line
This list is from smatch: grep -w "fit into 0" warns.txt | grep -v "\-1" | tee err-list
Also it removed some other real bugs as well, which I have listed below. It should have been: grep -w "fit into 0" warns.txt | grep -vw "\-1" | tee err-list
But you are right assigning -1 is idiomatic. I'll push a change tomorrow so the grep isn't needed.
regards,
dan carpenter
drivers/net/bnx2x_link.c +864 bnx2x_cl45_write(38) warn: value -14 can't fit into 0 'rc' drivers/net/bnx2x_link.c +884 bnx2x_cl45_write(58) warn: value -14 can't fit into 0 'rc' drivers/net/bnx2x_link.c +934 bnx2x_cl45_read(40) warn: value -14 can't fit into 0 'rc' drivers/net/bnx2x_link.c +957 bnx2x_cl45_read(63) warn: value -14 can't fit into 0 'rc' drivers/pci/hotplug/ibmphp_hpc.c +391 pci_ctrl_write(2) warn: value -19 can't fit into 0 'rc' drivers/usb/serial/iuu_phoenix.c +573 iuu_clk(133) warn: value -124 can't fit into 0 '*(priv->buf+Count++)' net/llc/llc_station.c +691 llc_station_init(2) warn: value -105 can't fit into 0 'rc' net/wireless/wext-compat.c +175 cfg80211_wext_giwrange(34) warn: value -110 can't fit into 0 'range->max_qual.level'