On 7/6/2017 12:23 AM, Leon Romanovsky wrote: > On Wed, Jul 05, 2017 at 05:17:52PM -0400, Doug Ledford wrote: > > From: Niranjana Vishwanathapura <niranjana.vishwanathapura@xxxxxxxx > > m> > > > > -static void mlx5_ib_free_rdma_netdev(struct net_device *netdev) > > -{ > > - return mlx5_rdma_netdev_free(netdev); > > + netdev = mlx5_rdma_netdev_alloc(to_mdev(hca)->mdev, hca, > > + name, setup); > > + if (likely(!IS_ERR_OR_NULL(netdev))) { > > + rn = netdev_priv(netdev); > > + rn->free_rdma_netdev = mlx5_ib_free_rdma_netdev; > > + } > > + return netdev; > > } > > > Thanks Doug, it looks good enough for the fix. > > In general, the "likely" is not needed here (we are not in data path) It doesn't hurt though... > and our preference is to avoid "if(!error) { do something }" > constructions > in favor of "if(error) { return ...}" (fail as early as you can). Normally I would agree with you on that point. But when you aren't returning an error code but instead are returning the same thing you return in the non error case, and when there are so few things to be done in the non error case, I think this sort of construct becomes more appealing (mainly because it will more closely match the assembler that GCC puts out when compiling this code and I think that has value for those times when you need to debug the object code, but that's just my personal opinion). > Reviewed-by: Leon Romanovsky <leonro@xxxxxxxxxxxx> > -- Doug Ledford <dledford@xxxxxxxxxx> GPG KeyID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD -- 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