[parent not found: <1442842450-29769-1-git-send-email-a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>]
* [PATCH 08/38] openvswitch: fix handling result of ipv6_skip_exthdr
[not found] ` <1442842450-29769-1-git-send-email-a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
@ 2015-09-21 13:33 ` Andrzej Hajda
2015-09-21 17:45 ` Pravin Shelar
0 siblings, 1 reply; 22+ messages in thread
From: Andrzej Hajda @ 2015-09-21 13:33 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: dev-yBygre7rU0TnMu66kgdUjQ, Bartlomiej Zolnierkiewicz,
netdev-u79uwXL29TY76Z2rM5mHXA, Andrzej Hajda, David S. Miller,
Marek Szyprowski
The function can return negative value.
The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
net/openvswitch/conntrack.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 002a755..fde3391 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -253,7 +253,7 @@ static int ovs_ct_helper(struct sk_buff *skb, u16 proto)
const struct nf_conntrack_helper *helper;
const struct nf_conn_help *help;
enum ip_conntrack_info ctinfo;
- unsigned int protoff;
+ int protoff;
struct nf_conn *ct;
ct = nf_ct_get(skb, &ctinfo);
--
1.9.1
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 08/38] openvswitch: fix handling result of ipv6_skip_exthdr
2015-09-21 13:33 ` [PATCH 08/38] openvswitch: fix handling result of ipv6_skip_exthdr Andrzej Hajda
@ 2015-09-21 17:45 ` Pravin Shelar
0 siblings, 0 replies; 22+ messages in thread
From: Pravin Shelar @ 2015-09-21 17:45 UTC (permalink / raw)
To: Andrzej Hajda
Cc: LKML, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
David S. Miller, netdev, dev@openvswitch.org
On Mon, Sep 21, 2015 at 6:33 AM, Andrzej Hajda <a.hajda@samsung.com> wrote:
> The function can return negative value.
>
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> net/openvswitch/conntrack.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 002a755..fde3391 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -253,7 +253,7 @@ static int ovs_ct_helper(struct sk_buff *skb, u16 proto)
> const struct nf_conntrack_helper *helper;
> const struct nf_conn_help *help;
> enum ip_conntrack_info ctinfo;
> - unsigned int protoff;
> + int protoff;
> struct nf_conn *ct;
>
A patch is already pushed to the net tree to fix this issue.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 17/38] isdn: hisax: fix frame calculation
2015-09-21 13:33 [PATCH 00/38] Fixes related to incorrect usage of unsigned types Andrzej Hajda
[not found] ` <1442842450-29769-1-git-send-email-a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
@ 2015-09-21 13:33 ` Andrzej Hajda
2015-09-22 23:15 ` David Miller
2015-09-21 13:33 ` [PATCH 18/38] net/ibm/emac: fix type of phy_mode Andrzej Hajda
` (7 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Andrzej Hajda @ 2015-09-21 13:33 UTC (permalink / raw)
To: linux-kernel
Cc: Andrzej Hajda, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
Karsten Keil, netdev
Difference of unsigned values is also unsigned so it does not make
sense to check its sign.
The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/isdn/hisax/hfc4s8s_l1.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/isdn/hisax/hfc4s8s_l1.c b/drivers/isdn/hisax/hfc4s8s_l1.c
index 0e5d673..9600cd7 100644
--- a/drivers/isdn/hisax/hfc4s8s_l1.c
+++ b/drivers/isdn/hisax/hfc4s8s_l1.c
@@ -646,14 +646,14 @@ rx_d_frame(struct hfc4s8s_l1 *l1p, int ech)
f1 = Read_hfc8_stable(l1p->hw, A_F1);
f2 = Read_hfc8(l1p->hw, A_F2);
- df = f1 - f2;
- if ((f1 - f2) < 0)
- df = f1 - f2 + MAX_F_CNT + 1;
+ if (f1 < f2)
+ df = MAX_F_CNT + 1 + f1 - f2;
+ else
+ df = f1 - f2;
- if (!df) {
+ if (!df)
return; /* no complete frame in fifo */
- }
z1 = Read_hfc16_stable(l1p->hw, A_Z1);
z2 = Read_hfc16(l1p->hw, A_Z2);
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 18/38] net/ibm/emac: fix type of phy_mode
2015-09-21 13:33 [PATCH 00/38] Fixes related to incorrect usage of unsigned types Andrzej Hajda
[not found] ` <1442842450-29769-1-git-send-email-a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2015-09-21 13:33 ` [PATCH 17/38] isdn: hisax: fix frame calculation Andrzej Hajda
@ 2015-09-21 13:33 ` Andrzej Hajda
2015-09-22 23:14 ` David Miller
2015-09-21 13:33 ` [PATCH 19/38] net: stmmac: fix type of entry variable Andrzej Hajda
` (6 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Andrzej Hajda @ 2015-09-21 13:33 UTC (permalink / raw)
To: linux-kernel
Cc: Andrzej Hajda, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
Ivan Mikhaylov, David S. Miller, netdev
phy_mode can be negative.
The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/net/ethernet/ibm/emac/core.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/emac/core.h b/drivers/net/ethernet/ibm/emac/core.h
index 28df374..f379e47 100644
--- a/drivers/net/ethernet/ibm/emac/core.h
+++ b/drivers/net/ethernet/ibm/emac/core.h
@@ -181,7 +181,7 @@ struct emac_instance {
struct mal_commac commac;
/* PHY infos */
- u32 phy_mode;
+ int phy_mode;
u32 phy_map;
u32 phy_address;
u32 phy_feat_exc;
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 19/38] net: stmmac: fix type of entry variable
2015-09-21 13:33 [PATCH 00/38] Fixes related to incorrect usage of unsigned types Andrzej Hajda
` (2 preceding siblings ...)
2015-09-21 13:33 ` [PATCH 18/38] net/ibm/emac: fix type of phy_mode Andrzej Hajda
@ 2015-09-21 13:33 ` Andrzej Hajda
2015-09-22 23:15 ` David Miller
2015-09-21 13:33 ` [PATCH 20/38] net: brcm80211: fix range check Andrzej Hajda
` (5 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Andrzej Hajda @ 2015-09-21 13:33 UTC (permalink / raw)
To: linux-kernel
Cc: Andrzej Hajda, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
Giuseppe Cavallaro, netdev
Variable can store negative values.
The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 925f2f8..934143e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1945,7 +1945,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct stmmac_priv *priv = netdev_priv(dev);
unsigned int txsize = priv->dma_tx_size;
- unsigned int entry;
+ int entry;
int i, csum_insertion = 0, is_jumbo = 0;
int nfrags = skb_shinfo(skb)->nr_frags;
struct dma_desc *desc, *first;
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 20/38] net: brcm80211: fix range check
2015-09-21 13:33 [PATCH 00/38] Fixes related to incorrect usage of unsigned types Andrzej Hajda
` (3 preceding siblings ...)
2015-09-21 13:33 ` [PATCH 19/38] net: stmmac: fix type of entry variable Andrzej Hajda
@ 2015-09-21 13:33 ` Andrzej Hajda
2015-09-22 23:15 ` David Miller
2015-09-21 13:33 ` [PATCH 21/38] mwifiex: fix comparison expression Andrzej Hajda
` (4 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Andrzej Hajda @ 2015-09-21 13:33 UTC (permalink / raw)
To: linux-kernel
Cc: Andrzej Hajda, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
Brett Rudley, Arend van Spriel, Franky (Zhenhui) Lin,
Hante Meuleman, Kalle Valo, linux-wireless, brcm80211-dev-list,
netdev
Unsigned minus constant is still unsigned so checking its sign makes no
sense.
The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/net/wireless/brcm80211/brcmsmac/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c
index 9728be0..218cbc8 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/main.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/main.c
@@ -4585,7 +4585,7 @@ static int brcms_b_attach(struct brcms_c_info *wlc, struct bcma_device *core,
wlc_hw->machwcap_backup = wlc_hw->machwcap;
/* init tx fifo size */
- WARN_ON((wlc_hw->corerev - XMTFIFOTBL_STARTREV) < 0 ||
+ WARN_ON(wlc_hw->corerev < XMTFIFOTBL_STARTREV ||
(wlc_hw->corerev - XMTFIFOTBL_STARTREV) >
ARRAY_SIZE(xmtfifo_sz));
wlc_hw->xmtfifo_sz =
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 20/38] net: brcm80211: fix range check
2015-09-21 13:33 ` [PATCH 20/38] net: brcm80211: fix range check Andrzej Hajda
@ 2015-09-22 23:15 ` David Miller
0 siblings, 0 replies; 22+ messages in thread
From: David Miller @ 2015-09-22 23:15 UTC (permalink / raw)
To: a.hajda
Cc: linux-kernel, b.zolnierkie, m.szyprowski, brudley, arend, frankyl,
meuleman, kvalo, linux-wireless, brcm80211-dev-list, netdev
From: Andrzej Hajda <a.hajda@samsung.com>
Date: Mon, 21 Sep 2015 15:33:52 +0200
> Unsigned minus constant is still unsigned so checking its sign makes no
> sense.
>
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Applied.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 21/38] mwifiex: fix comparison expression
2015-09-21 13:33 [PATCH 00/38] Fixes related to incorrect usage of unsigned types Andrzej Hajda
` (4 preceding siblings ...)
2015-09-21 13:33 ` [PATCH 20/38] net: brcm80211: fix range check Andrzej Hajda
@ 2015-09-21 13:33 ` Andrzej Hajda
2015-09-22 10:56 ` Amitkumar Karwar
2015-09-22 23:15 ` David Miller
2015-09-21 13:33 ` [PATCH 22/38] orinoco: fix checking for default value Andrzej Hajda
` (3 subsequent siblings)
9 siblings, 2 replies; 22+ messages in thread
From: Andrzej Hajda @ 2015-09-21 13:33 UTC (permalink / raw)
To: linux-kernel
Cc: Andrzej Hajda, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
Amitkumar Karwar, Nishant Sarmukadam, Kalle Valo, linux-wireless,
netdev
To avoid underflows signed variables should be used in expression.
The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/net/wireless/mwifiex/11n_rxreorder.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/11n_rxreorder.c b/drivers/net/wireless/mwifiex/11n_rxreorder.c
index 2906cd5..b3970a8 100644
--- a/drivers/net/wireless/mwifiex/11n_rxreorder.c
+++ b/drivers/net/wireless/mwifiex/11n_rxreorder.c
@@ -615,10 +615,10 @@ int mwifiex_11n_rx_reorder_pkt(struct mwifiex_private *priv,
((end_win > start_win) && ((seq_num > end_win) ||
(seq_num < start_win)))) {
end_win = seq_num;
- if (((seq_num - win_size) + 1) >= 0)
+ if (((end_win - win_size) + 1) >= 0)
start_win = (end_win - win_size) + 1;
else
- start_win = (MAX_TID_VALUE - (win_size - seq_num)) + 1;
+ start_win = (MAX_TID_VALUE - (win_size - end_win)) + 1;
mwifiex_11n_dispatch_pkt_until_start_win(priv, tbl, start_win);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* RE: [PATCH 21/38] mwifiex: fix comparison expression
2015-09-21 13:33 ` [PATCH 21/38] mwifiex: fix comparison expression Andrzej Hajda
@ 2015-09-22 10:56 ` Amitkumar Karwar
2015-09-22 23:15 ` David Miller
1 sibling, 0 replies; 22+ messages in thread
From: Amitkumar Karwar @ 2015-09-22 10:56 UTC (permalink / raw)
To: Andrzej Hajda, linux-kernel@vger.kernel.org
Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Nishant Sarmukadam,
Kalle Valo, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org
Hi Andrzej,
> From: Andrzej Hajda [mailto:a.hajda@samsung.com]
> Sent: Monday, September 21, 2015 7:04 PM
> To: linux-kernel@vger.kernel.org
> Cc: Andrzej Hajda; Bartlomiej Zolnierkiewicz; Marek Szyprowski;
> Amitkumar Karwar; Nishant Sarmukadam; Kalle Valo; linux-
> wireless@vger.kernel.org; netdev@vger.kernel.org
> Subject: [PATCH 21/38] mwifiex: fix comparison expression
>
> To avoid underflows signed variables should be used in expression.
>
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> drivers/net/wireless/mwifiex/11n_rxreorder.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/mwifiex/11n_rxreorder.c
> b/drivers/net/wireless/mwifiex/11n_rxreorder.c
> index 2906cd5..b3970a8 100644
> --- a/drivers/net/wireless/mwifiex/11n_rxreorder.c
> +++ b/drivers/net/wireless/mwifiex/11n_rxreorder.c
> @@ -615,10 +615,10 @@ int mwifiex_11n_rx_reorder_pkt(struct
> mwifiex_private *priv,
> ((end_win > start_win) && ((seq_num > end_win) ||
> (seq_num < start_win)))) {
> end_win = seq_num;
> - if (((seq_num - win_size) + 1) >= 0)
> + if (((end_win - win_size) + 1) >= 0)
> start_win = (end_win - win_size) + 1;
> else
> - start_win = (MAX_TID_VALUE - (win_size - seq_num)) +
> 1;
> + start_win = (MAX_TID_VALUE - (win_size - end_win)) +
> 1;
> mwifiex_11n_dispatch_pkt_until_start_win(priv, tbl,
> start_win);
> }
>
Acked-by: Amitkumar Karwar <akarwar@marvell.com>
Regards,
Amitkumar
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 21/38] mwifiex: fix comparison expression
2015-09-21 13:33 ` [PATCH 21/38] mwifiex: fix comparison expression Andrzej Hajda
2015-09-22 10:56 ` Amitkumar Karwar
@ 2015-09-22 23:15 ` David Miller
1 sibling, 0 replies; 22+ messages in thread
From: David Miller @ 2015-09-22 23:15 UTC (permalink / raw)
To: a.hajda
Cc: linux-kernel, b.zolnierkie, m.szyprowski, akarwar, nishants,
kvalo, linux-wireless, netdev
From: Andrzej Hajda <a.hajda@samsung.com>
Date: Mon, 21 Sep 2015 15:33:53 +0200
> To avoid underflows signed variables should be used in expression.
>
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Applied.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 22/38] orinoco: fix checking for default value
2015-09-21 13:33 [PATCH 00/38] Fixes related to incorrect usage of unsigned types Andrzej Hajda
` (5 preceding siblings ...)
2015-09-21 13:33 ` [PATCH 21/38] mwifiex: fix comparison expression Andrzej Hajda
@ 2015-09-21 13:33 ` Andrzej Hajda
2015-09-22 23:15 ` David Miller
2015-09-21 13:33 ` [PATCH 23/38] rndis_wlan: " Andrzej Hajda
` (2 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Andrzej Hajda @ 2015-09-21 13:33 UTC (permalink / raw)
To: linux-kernel
Cc: Andrzej Hajda, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
Kalle Valo, linux-wireless, netdev
Thresholds uses -1 to indicate that default value should be used.
Since thresholds are unsigned sign checking makes no sense.
The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/net/wireless/orinoco/cfg.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/orinoco/cfg.c b/drivers/net/wireless/orinoco/cfg.c
index a9e94b6..0f6ea31 100644
--- a/drivers/net/wireless/orinoco/cfg.c
+++ b/drivers/net/wireless/orinoco/cfg.c
@@ -220,7 +220,7 @@ static int orinoco_set_wiphy_params(struct wiphy *wiphy, u32 changed)
if (changed & WIPHY_PARAM_FRAG_THRESHOLD) {
/* Set fragmentation */
if (priv->has_mwo) {
- if (wiphy->frag_threshold < 0)
+ if (wiphy->frag_threshold == -1)
frag_value = 0;
else {
printk(KERN_WARNING "%s: Fixed fragmentation "
@@ -230,7 +230,7 @@ static int orinoco_set_wiphy_params(struct wiphy *wiphy, u32 changed)
frag_value = 1;
}
} else {
- if (wiphy->frag_threshold < 0)
+ if (wiphy->frag_threshold == -1)
frag_value = 2346;
else if ((wiphy->frag_threshold < 257) ||
(wiphy->frag_threshold > 2347))
@@ -252,7 +252,7 @@ static int orinoco_set_wiphy_params(struct wiphy *wiphy, u32 changed)
* the upper limit.
*/
- if (wiphy->rts_threshold < 0)
+ if (wiphy->rts_threshold == -1)
rts_value = 2347;
else if (wiphy->rts_threshold > 2347)
err = -EINVAL;
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 23/38] rndis_wlan: fix checking for default value
2015-09-21 13:33 [PATCH 00/38] Fixes related to incorrect usage of unsigned types Andrzej Hajda
` (6 preceding siblings ...)
2015-09-21 13:33 ` [PATCH 22/38] orinoco: fix checking for default value Andrzej Hajda
@ 2015-09-21 13:33 ` Andrzej Hajda
2015-09-22 23:15 ` David Miller
2015-09-21 13:33 ` [PATCH 27/38] usbnet: remove invalid check Andrzej Hajda
[not found] ` <17571.1442842945@warthog.procyon.org.uk>
9 siblings, 1 reply; 22+ messages in thread
From: Andrzej Hajda @ 2015-09-21 13:33 UTC (permalink / raw)
To: linux-kernel
Cc: Andrzej Hajda, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
Jussi Kivilinna, Kalle Valo, linux-wireless, netdev
Thresholds uses -1 to indicate that default value should be used.
Since thresholds are unsigned sign checking makes no sense.
The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/net/wireless/rndis_wlan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index 71a825c..a13d1f2 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -1236,7 +1236,7 @@ static int set_rts_threshold(struct usbnet *usbdev, u32 rts_threshold)
netdev_dbg(usbdev->net, "%s(): %i\n", __func__, rts_threshold);
- if (rts_threshold < 0 || rts_threshold > 2347)
+ if (rts_threshold == -1 || rts_threshold > 2347)
rts_threshold = 2347;
tmp = cpu_to_le32(rts_threshold);
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 27/38] usbnet: remove invalid check
2015-09-21 13:33 [PATCH 00/38] Fixes related to incorrect usage of unsigned types Andrzej Hajda
` (7 preceding siblings ...)
2015-09-21 13:33 ` [PATCH 23/38] rndis_wlan: " Andrzej Hajda
@ 2015-09-21 13:33 ` Andrzej Hajda
2015-09-22 23:15 ` David Miller
[not found] ` <17571.1442842945@warthog.procyon.org.uk>
9 siblings, 1 reply; 22+ messages in thread
From: Andrzej Hajda @ 2015-09-21 13:33 UTC (permalink / raw)
To: linux-kernel
Cc: Andrzej Hajda, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
Steve Glendinning, David S. Miller, Woojung.Huh@microchip.com,
linux-usb, netdev
skb->len is always non-negative.
The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/net/usb/lan78xx.c | 5 -----
drivers/net/usb/smsc75xx.c | 5 -----
drivers/net/usb/smsc95xx.c | 5 -----
3 files changed, 15 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index a39518f..e0556dc 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2522,11 +2522,6 @@ static int lan78xx_rx(struct lan78xx_net *dev, struct sk_buff *skb)
skb_pull(skb, align_count);
}
- if (unlikely(skb->len < 0)) {
- netdev_warn(dev->net, "invalid rx length<0 %d", skb->len);
- return 0;
- }
-
return 1;
}
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index d9e7892..30033db 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -2185,11 +2185,6 @@ static int smsc75xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
skb_pull(skb, align_count);
}
- if (unlikely(skb->len < 0)) {
- netdev_warn(dev->net, "invalid rx length<0 %d\n", skb->len);
- return 0;
- }
-
return 1;
}
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 26423ad..66b3ab9 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1815,11 +1815,6 @@ static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
skb_pull(skb, align_count);
}
- if (unlikely(skb->len < 0)) {
- netdev_warn(dev->net, "invalid rx length<0 %d\n", skb->len);
- return 0;
- }
-
return 1;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread[parent not found: <17571.1442842945@warthog.procyon.org.uk>]
* Re: [PATCH 00/38] Fixes related to incorrect usage of unsigned types
[not found] ` <17571.1442842945@warthog.procyon.org.uk>
@ 2015-09-22 9:13 ` Andrzej Hajda
2015-09-22 9:46 ` Jacek Anaszewski
0 siblings, 1 reply; 22+ messages in thread
From: Andrzej Hajda @ 2015-09-22 9:13 UTC (permalink / raw)
To: David Howells
Cc: linux-mips, linux-fbdev, linux-sh, brcm80211-dev-list, dri-devel,
linux-mm, linux-clk, linux-leds, Marek Szyprowski, devel,
linux-rdma, linux-cachefs, linux-serial, linux-input, linux-media,
dev, rtc-linux, Bartlomiej Zolnierkiewicz, intel-gfx, linux-omap,
linux-arm-kernel, Andrzej Hajda, linux-api, linux-usb,
linux-wireless, linux-kernel, linux-crypto, netdev, lustre-devel
On 09/21/2015 03:42 PM, David Howells wrote:
> Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>
>> Semantic patch finds comparisons of types:
>> unsigned < 0
>> unsigned >= 0
>> The former is always false, the latter is always true.
>> Such comparisons are useless, so theoretically they could be
>> safely removed, but their presence quite often indicates bugs.
>
> Or someone has left them in because they don't matter and there's the
> possibility that the type being tested might be or become signed under some
> circumstances. If the comparison is useless, I'd expect the compiler to just
> discard it - for such cases your patch is pointless.
>
> If I have, for example:
>
> unsigned x;
>
> if (x == 0 || x > 27)
> give_a_range_error();
>
> I will write this as:
>
> unsigned x;
>
> if (x <= 0 || x > 27)
> give_a_range_error();
>
> because it that gives a way to handle x being changed to signed at some point
> in the future for no cost. In which case, your changing the <= to an ==
> "because the < part of the case is useless" is arguably wrong.
This is why I have not checked for such cases - I have skipped checks of type
unsigned <= 0
exactly for the reasons above.
However I have left two other checks as they seems to me more suspicious - they
are always true or false. But as Dmitry and Andrew pointed out Linus have quite
strong opinion against removing range checks in such cases as he finds it
clearer. I think it applies to patches 29-36. I am not sure about patches 26-28,37.
Regards
Andrzej
>
> David
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 00/38] Fixes related to incorrect usage of unsigned types
2015-09-22 9:13 ` [PATCH 00/38] Fixes related to incorrect usage of unsigned types Andrzej Hajda
@ 2015-09-22 9:46 ` Jacek Anaszewski
0 siblings, 0 replies; 22+ messages in thread
From: Jacek Anaszewski @ 2015-09-22 9:46 UTC (permalink / raw)
To: Andrzej Hajda
Cc: David Howells, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
linux-kernel, brcm80211-dev-list, devel, dev, dri-devel,
intel-gfx, linux-api, linux-arm-kernel, linux-cachefs, linux-clk,
linux-crypto, linux-fbdev, linux-input, linux-leds, linux-media,
linux-mips, linux-mm, linux-omap, linux-rdma, linux-serial,
linux-sh, linux-usb, linux-wireless, lustre-devel, netdev,
rtc-linux
On 09/22/2015 11:13 AM, Andrzej Hajda wrote:
> On 09/21/2015 03:42 PM, David Howells wrote:
>> Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>>
>>> Semantic patch finds comparisons of types:
>>> unsigned < 0
>>> unsigned >= 0
>>> The former is always false, the latter is always true.
>>> Such comparisons are useless, so theoretically they could be
>>> safely removed, but their presence quite often indicates bugs.
>>
>> Or someone has left them in because they don't matter and there's the
>> possibility that the type being tested might be or become signed under some
>> circumstances. If the comparison is useless, I'd expect the compiler to just
>> discard it - for such cases your patch is pointless.
>>
>> If I have, for example:
>>
>> unsigned x;
>>
>> if (x == 0 || x > 27)
>> give_a_range_error();
>>
>> I will write this as:
>>
>> unsigned x;
>>
>> if (x <= 0 || x > 27)
>> give_a_range_error();
>>
>> because it that gives a way to handle x being changed to signed at some point
>> in the future for no cost. In which case, your changing the <= to an ==
>> "because the < part of the case is useless" is arguably wrong.
>
> This is why I have not checked for such cases - I have skipped checks of type
> unsigned <= 0
> exactly for the reasons above.
>
> However I have left two other checks as they seems to me more suspicious - they
> are always true or false. But as Dmitry and Andrew pointed out Linus have quite
> strong opinion against removing range checks in such cases as he finds it
> clearer. I think it applies to patches 29-36. I am not sure about patches 26-28,37.
Dropped 30/38 and 31/38 from LED tree then.
--
Best Regards,
Jacek Anaszewski
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 22+ messages in thread