Re: [PATCH v4 resend 04/23] ice: Register auxiliary device to provide RDMA

[Jason Gunthorpe <jgg@nvidia.com>]

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.

Jason