| Message ID | 1340177259-14083-1-git-send-email-santoshprasadnayak@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, Jun 20, 2012 at 12:57:39PM +0530, santosh nayak wrote: > From: Santosh Nayak <santoshprasadnayak@gmail.com> > > There are 'NETXEN_NIU_MAX_GBE_PORTS' GBE ports. Port indexing starts > from zero. > Hence we should also return error for "port == NETXEN_NIU_MAX_GBE_PORTS" > I don't know this code well enough to say if you are right or not, but what about for port == NETXEN_NIU_MAX_XG_PORTS a few lines later in both functions? regards, dan carpenter -- 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 Wed, Jun 20, 2012 at 12:57:39PM +0530, santosh nayak wrote: > From: Santosh Nayak <santoshprasadnayak@gmail.com> > > There are 'NETXEN_NIU_MAX_GBE_PORTS' GBE ports. Port indexing starts > from zero. > Hence we should also return error for "port == NETXEN_NIU_MAX_GBE_PORTS" > So my understanding is that you are guessing on this based on the fact that, "Who counts from 0-4 inclusive?". I can't argue with that logic. Looking some more at NETXEN_NIU_MAX_XG_PORTS. It is used off by one 4 times and it's used correctly 1 time. regards, dan carpenter -- 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 Wed, Jun 20, 2012 at 1:14 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote: > On Wed, Jun 20, 2012 at 12:57:39PM +0530, santosh nayak wrote: >> From: Santosh Nayak <santoshprasadnayak@gmail.com> >> >> There are 'NETXEN_NIU_MAX_GBE_PORTS' GBE ports. Port indexing starts >> from zero. >> Hence we should also return error for "port == NETXEN_NIU_MAX_GBE_PORTS" >> > > I don't know this code well enough to say if you are right or not, > but what about for port == NETXEN_NIU_MAX_XG_PORTS a few lines later > in both functions? I think "for port == NETXEN_NIU_MAX_XG_PORTS" error should be returned. @Rajesh, Can you please comment on it ? regards santosh > > regards, > dan carpenter > -- 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 Wed, Jun 20, 2012 at 3:21 PM, Rajesh Borundia <rajesh.borundia@qlogic.com> wrote: > _______________________________________ > From: santosh prasad nayak [santoshprasadnayak@gmail.com] > Sent: Wednesday, June 20, 2012 1:29 PM > To: Dan Carpenter; Rajesh Borundia > Cc: Sony Chacko; netdev; kernel-janitors@vger.kernel.org > Subject: Re: [PATCH] netxen: Error return off by one in 'netxen_nic_set_pauseparam()'. > > On Wed, Jun 20, 2012 at 1:14 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote: >> On Wed, Jun 20, 2012 at 12:57:39PM +0530, santosh nayak wrote: >>> From: Santosh Nayak <santoshprasadnayak@gmail.com> >>> >>> There are 'NETXEN_NIU_MAX_GBE_PORTS' GBE ports. Port indexing starts >>> from zero. >>> Hence we should also return error for "port == NETXEN_NIU_MAX_GBE_PORTS" >>> >> >> I don't know this code well enough to say if you are right or not, >> but what about for port == NETXEN_NIU_MAX_XG_PORTS a few lines later >> in both functions? > > > I think "for port == NETXEN_NIU_MAX_XG_PORTS" error should be returned. > > > @Rajesh, > > Can you please comment on it ? > > > regards > santosh > >> >> regards, >> dan carpenter >> > > Yes error should be returned for both port == NETXEN_NIU_MAX_XG_PORTS and > port == NETXEN_NIU_MAX_GBE_PORTS. Ok. The current patch is for GBE port. For XG port I will send another patch. regards santosh > > > Rajesh > -- 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
From: santosh nayak <santoshprasadnayak@gmail.com> Date: Wed, 20 Jun 2012 12:57:39 +0530 > From: Santosh Nayak <santoshprasadnayak@gmail.com> > > There are 'NETXEN_NIU_MAX_GBE_PORTS' GBE ports. Port indexing starts > from zero. > Hence we should also return error for "port == NETXEN_NIU_MAX_GBE_PORTS" > > Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com> Applied. -- 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/qlogic/netxen/netxen_nic_ethtool.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_ethtool.c index 3973040..d4f179f 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_ethtool.c +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_ethtool.c @@ -489,7 +489,7 @@ netxen_nic_get_pauseparam(struct net_device *dev, int port = adapter->physical_port; if (adapter->ahw.port_type == NETXEN_NIC_GBE) { - if ((port < 0) || (port > NETXEN_NIU_MAX_GBE_PORTS)) + if ((port < 0) || (port >= NETXEN_NIU_MAX_GBE_PORTS)) return; /* get flow control settings */ val = NXRD32(adapter, NETXEN_NIU_GB_MAC_CONFIG_0(port)); @@ -534,7 +534,7 @@ netxen_nic_set_pauseparam(struct net_device *dev, int port = adapter->physical_port; /* read mode */ if (adapter->ahw.port_type == NETXEN_NIC_GBE) { - if ((port < 0) || (port > NETXEN_NIU_MAX_GBE_PORTS)) + if ((port < 0) || (port >= NETXEN_NIU_MAX_GBE_PORTS)) return -EIO; /* set flow control */ val = NXRD32(adapter, NETXEN_NIU_GB_MAC_CONFIG_0(port));