| Message ID | 1446758751-27999-5-git-send-email-mchan@broadcom.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
Hello. On 11/6/2015 12:25 AM, Michael Chan wrote: > Assign the return value from bitmap_find_free_region() to an integer > variable and check for negative error codes first, before assigning > the bit ID to the unsigned sw_id field. > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Cc: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Michael Chan <mchan@broadcom.com> > --- > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c > index a62deff..db15c5e 100644 > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c > @@ -5307,7 +5307,7 @@ static int bnxt_rx_flow_steer(struct net_device *dev, const struct sk_buff *skb, > struct bnxt_ntuple_filter *fltr, *new_fltr; > struct flow_keys *fkeys; > struct ethhdr *eth = (struct ethhdr *)skb_mac_header(skb); > - int rc = 0, idx; > + int rc = 0, idx, bit_id; I'd use the already declared 'rc' variable. [...] MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2015-11-06 at 14:57 +0300, Sergei Shtylyov wrote: > Hello. > > On 11/6/2015 12:25 AM, Michael Chan wrote: > > > Assign the return value from bitmap_find_free_region() to an integer > > variable and check for negative error codes first, before assigning > > the bit ID to the unsigned sw_id field. > > > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > Cc: Dan Carpenter <dan.carpenter@oracle.com> > > Signed-off-by: Michael Chan <mchan@broadcom.com> > > --- > > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c > > index a62deff..db15c5e 100644 > > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c > > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c > > @@ -5307,7 +5307,7 @@ static int bnxt_rx_flow_steer(struct net_device *dev, const struct sk_buff *skb, > > struct bnxt_ntuple_filter *fltr, *new_fltr; > > struct flow_keys *fkeys; > > struct ethhdr *eth = (struct ethhdr *)skb_mac_header(skb); > > - int rc = 0, idx; > > + int rc = 0, idx, bit_id; > > I'd use the already declared 'rc' variable. > I agree with you. Normally, rc should only contain 0 or negative error codes and is returned to the caller. In this particular case, the caller expects negative or non negative values and we could have used rc. Perhaps I'll make the change in the next round of patches. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index a62deff..db15c5e 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -5307,7 +5307,7 @@ static int bnxt_rx_flow_steer(struct net_device *dev, const struct sk_buff *skb, struct bnxt_ntuple_filter *fltr, *new_fltr; struct flow_keys *fkeys; struct ethhdr *eth = (struct ethhdr *)skb_mac_header(skb); - int rc = 0, idx; + int rc = 0, idx, bit_id; struct hlist_head *head; if (skb->encapsulation) @@ -5345,14 +5345,15 @@ static int bnxt_rx_flow_steer(struct net_device *dev, const struct sk_buff *skb, rcu_read_unlock(); spin_lock_bh(&bp->ntp_fltr_lock); - new_fltr->sw_id = bitmap_find_free_region(bp->ntp_fltr_bmap, - BNXT_NTP_FLTR_MAX_FLTR, 0); - if (new_fltr->sw_id < 0) { + bit_id = bitmap_find_free_region(bp->ntp_fltr_bmap, + BNXT_NTP_FLTR_MAX_FLTR, 0); + if (bit_id < 0) { spin_unlock_bh(&bp->ntp_fltr_lock); rc = -ENOMEM; goto err_free; } + new_fltr->sw_id = (u16)bit_id; new_fltr->flow_id = flow_id; new_fltr->rxq = rxq_index; hlist_add_head_rcu(&new_fltr->hash, head);
Assign the return value from bitmap_find_free_region() to an integer variable and check for negative error codes first, before assigning the bit ID to the unsigned sw_id field. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Cc: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Michael Chan <mchan@broadcom.com> --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)