On 11/16/2025 7:56 AM, Ally Heev wrote: > usage of cleanup attributes is discouraged in net [1], achieve cleanup > using goto. In this patch though, only uninitialized pointers with __free > attribute are cleaned as they can cause undefined behavior when they > go out of scope > > Suggested-by: Simon Horman <[email protected]> > Reported-by: Dan Carpenter <[email protected]> > Closes: https://lore.kernel.org/all/[email protected]/ > Signed-off-by: Ally Heev <[email protected]> > > [1] > https://docs.kernel.org/process/maintainer-netdev.html#using-device-managed-and-cleanup-h-constructs
<SMH> If using __free() *correctly* gives more readable code, why wouldn't we do it? The only problem with this function is that it doesn't follow the guidance from cleanup.h: * the recommendation is to always define and assign variables in one * statement and not group variable definitions at the top of the * function when __free() is used. > > Signed-off-by: Ally Heev <[email protected]> > --- > drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 28 > +++++++++++++++++-------- > 1 file changed, 19 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c > b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c > index > cbb5fa30f5a0ec778c1ee30470da3ca21cc1af24..5b2bf8c3205bc1ea0746f78afa2a24f3f8ad2a8c > 100644 > --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c > +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c > @@ -1012,7 +1012,7 @@ static int idpf_send_get_caps_msg(struct idpf_adapter > *adapter) > */ > static int idpf_send_get_lan_memory_regions(struct idpf_adapter *adapter) > { > - struct virtchnl2_get_lan_memory_regions *rcvd_regions __free(kfree); > + struct virtchnl2_get_lan_memory_regions *rcvd_regions = NULL; > struct idpf_vc_xn_params xn_params = { > .vc_op = VIRTCHNL2_OP_GET_LAN_MEMORY_REGIONS, > .recv_buf.iov_len = IDPF_CTLQ_MAX_BUF_LEN, > @@ -1029,21 +1029,29 @@ static int idpf_send_get_lan_memory_regions(struct > idpf_adapter *adapter) > > xn_params.recv_buf.iov_base = rcvd_regions; > reply_sz = idpf_vc_xn_exec(adapter, &xn_params); > - if (reply_sz < 0) > - return reply_sz; > + if (reply_sz < 0) { > + err = reply_sz; > + goto out; > + } > > num_regions = le16_to_cpu(rcvd_regions->num_memory_regions); > size = struct_size(rcvd_regions, mem_reg, num_regions); > - if (reply_sz < size) > - return -EIO; > + if (reply_sz < size) { > + err = -EIO; > + goto out; > + } > > - if (size > IDPF_CTLQ_MAX_BUF_LEN) > - return -EINVAL; > + if (size > IDPF_CTLQ_MAX_BUF_LEN) { > + err = -EINVAL; > + goto out; > + } > > hw = &adapter->hw; > hw->lan_regs = kcalloc(num_regions, sizeof(*hw->lan_regs), GFP_KERNEL); > - if (!hw->lan_regs) > - return -ENOMEM; > + if (!hw->lan_regs) { > + err = -ENOMEM; > + goto out; > + } > > for (int i = 0; i < num_regions; i++) { > hw->lan_regs[i].addr_len = > @@ -1053,6 +1061,8 @@ static int idpf_send_get_lan_memory_regions(struct > idpf_adapter *adapter) > } > hw->num_lan_regs = num_regions; > > +out: > + kfree(rcvd_regions); > return err; > } > >
