| Message ID | 20240712185431.81805-1-kiryushin@ancud.ru (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] bnx2x: remove redundant NULL-pointer check | expand |
On Fri, Jul 12, 2024 at 09:54:31PM +0300, Nikita Kiryushin wrote: > bnx2x_get_vf_config() contains NULL-pointer checks for > mac_obj and vlan_obj. > > The fields checked are assigned to (after macro expansions): > > mac_obj = &((vf)->vfqs[0].mac_obj); > vlan_obj = &((vf)->vfqs[0].vlan_obj); > > It is impossible to get NULL for those Hi Nikita, I agree with the above. > (and (vf)->vfqs was > checked earlier in bnx2x_vf_op_prep). But, FWIIW, I don't think the test on the two lines above is relevant. bnx2x_vf_op_prep does, conditionally, check that (vf)->vfqs is not NULL. But if (vf)->vfqs was null in the code you are updating (and I'm not saying it can be, just if it was), then neither mac_obj nor vlan_obj would be NULL due to the layout of struct bnx2x_vf_queue. > Remove superfluous NULL-pointer check and associated > unreachable code to improve readability. I also agree with this. > Signed-off-by: Nikita Kiryushin <kiryushin@ancud.ru> ...
I agree, I guess I was meaning to state, that bnx2x_vf_op_prep() already contains all the needed checks On 7/13/24 21:29, Simon Horman wrote: > But, FWIIW, I don't think the test on the two lines above is relevant. > > bnx2x_vf_op_prep does, conditionally, check that (vf)->vfqs is not NULL. > But if (vf)->vfqs was null in the code you are updating > (and I'm not saying it can be, just if it was), > then neither mac_obj nor vlan_obj would be NULL due to the > layout of struct bnx2x_vf_queue. >
On Mon, Jul 15, 2024 at 04:52:40PM +0300, Nikita Kiryushin wrote: > I agree, I guess I was meaning to state, that bnx2x_vf_op_prep() > already contains all the needed checks Thanks, I agree with that. Though if it would me I'd just drop the reference to bnx2x_vf_op_prep entirely.
On 7/15/24 21:10, Simon Horman wrote: > Though if it would me I'd just drop the reference > to bnx2x_vf_op_prep entirely. > Thank you for the feedback! I guess I will do so in second version when net-next open next time
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c index 77d4cb4ad782..3415bbe722a8 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c @@ -2619,10 +2619,6 @@ int bnx2x_get_vf_config(struct net_device *dev, int vfidx, mac_obj = &bnx2x_leading_vfq(vf, mac_obj); vlan_obj = &bnx2x_leading_vfq(vf, vlan_obj); - if (!mac_obj || !vlan_obj) { - BNX2X_ERR("VF partially initialized\n"); - return -EINVAL; - } ivi->vf = vfidx; ivi->qos = 0;
bnx2x_get_vf_config() contains NULL-pointer checks for mac_obj and vlan_obj. The fields checked are assigned to (after macro expansions): mac_obj = &((vf)->vfqs[0].mac_obj); vlan_obj = &((vf)->vfqs[0].vlan_obj); It is impossible to get NULL for those (and (vf)->vfqs was checked earlier in bnx2x_vf_op_prep). Remove superfluous NULL-pointer check and associated unreachable code to improve readability. Signed-off-by: Nikita Kiryushin <kiryushin@ancud.ru> --- drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 4 ---- 1 file changed, 4 deletions(-)