On Mon, Jun 05, 2017 at 03:54:15PM +0300, Leon Romanovsky wrote:
> 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.
Yeah, it is kind of confusing in first glance :)
It is the way how diff works. If makes it looks like that "if (!rc) is part
of the "else".
>
> Thanks,
> Reviewed-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
