Re: [PATCH] IB/bnxt_re: Check return value from get_link_ksettings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 05, 2017 at 03:52:27PM +0300, Leon Romanovsky wrote:
> On Mon, Jun 05, 2017 at 12:14:29PM +0300, Yuval Shaia wrote:
> > The function get_link_ksettings might return bad status indicating a
> > failure to retrieve interface atttibutes.
> > Check return value to cover this case.
> >
> > While there, change the zero-initialization to "compiler-helper" instead
> > of an expensive call to memcpy.
> >
> > Signed-off-by: Yuval Shaia <yuval.shaia@xxxxxxxxxx>
> > ---
> >  drivers/infiniband/hw/bnxt_re/ib_verbs.c | 16 +++++++++-------
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > index 7ba9e69..10c7189 100644
> > --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > @@ -183,17 +183,19 @@ int bnxt_re_modify_device(struct ib_device *ibdev,
> >
> >  static void __to_ib_speed_width(struct net_device *netdev, u8 *speed, u8 *width)
> >  {
> > -	struct ethtool_link_ksettings lksettings;
> > -	u32 espeed;
> > +	u32 espeed = SPEED_UNKNOWN;
> >
> >  	if (netdev->ethtool_ops && netdev->ethtool_ops->get_link_ksettings) {
> > -		memset(&lksettings, 0, sizeof(lksettings));
> > +		struct ethtool_link_ksettings lksettings = {0};
> > +		int rc;
> > +
> >  		rtnl_lock();
> > -		netdev->ethtool_ops->get_link_ksettings(netdev, &lksettings);
> > +		rc = netdev->ethtool_ops->get_link_ksettings(netdev,
> > +							     &lksettings);
> >  		rtnl_unlock();
> > -		espeed = lksettings.base.speed;
> > -	} else {
> > -		espeed = SPEED_UNKNOWN;
> > +
> > +		if (!rc)
>
> Are you sure that it is "if (!rc)" and not "if (rc)"?
> in commit message you wrote that "The function get_link_ksettings might
> return bad status indicating".

Sorry, you are right.

Thanks,
Reviewed-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux