> Subject: Re: [PATCH v4 resend 04/23] ice: Register auxiliary device to provide > RDMA > > On Tue, Apr 06, 2021 at 07:14:43PM -0500, Shiraz Saleem wrote: > > +/** > > + * ice_plug_aux_devs - allocate and register one AUX dev per > > +cdev_info in PF > > + * @pf: pointer to PF struct > > + */ > > +int ice_plug_aux_devs(struct ice_pf *pf) { > > + struct iidc_auxiliary_dev *iadev; > > + int ret, i; > > + > > + if (!pf->cdev_infos) > > + return 0; > > + > > + for (i = 0; i < ARRAY_SIZE(ice_cdev_ids); i++) { > > + struct iidc_core_dev_info *cdev_info; > > + struct auxiliary_device *adev; > > + > > + cdev_info = pf->cdev_infos[i]; > > + if (!cdev_info) > > + continue; > > + > > + iadev = kzalloc(sizeof(*iadev), GFP_KERNEL); > > + if (!iadev) > > + return -ENOMEM; > > + > > + adev = &iadev->adev; > > + cdev_info->adev = adev; > > + iadev->cdev_info = cdev_info; > > + > > + if (ice_cdev_ids[i].id == IIDC_RDMA_ID) { > > + if (cdev_info->rdma_protocol == > > + IIDC_RDMA_PROTOCOL_IWARP) > > + adev->name = kasprintf(GFP_KERNEL, "%s_%s", > > + ice_cdev_ids[i].name, > > + "iwarp"); > > + else > > + adev->name = kasprintf(GFP_KERNEL, "%s_%s", > > + ice_cdev_ids[i].name, > > + "roce"); > > + } else { > > + adev->name = kasprintf(GFP_KERNEL, "%s", > > + ice_cdev_ids[i].name); > > This never happens, it is dead code, right? > > > + } > > This is all confused, the adev->name is supposed to be a compile time constant, > this not be allocating memory and combining the constant "intel_rdma" and the > trailing "iwarp" "roce" > > Just use the proper define right here. > > Also all these kasprintfs should have their errors checked, which is also why it > shouldn't be written like this. > Agree to all the feedback here.