netdev - Re: [PATCH v1] net: wwan: t7xx: Add fastboot interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231221175332.GD1202958@kernel.org>
Date: Thu, 21 Dec 2023 18:53:32 +0100
From: Simon Horman <horms@...nel.org>
To: Jinjian Song <songjinjian@...mail.com>
Cc: netdev@...r.kernel.org, chandrashekar.devegowda@...el.com,
	chiranjeevi.rapolu@...ux.intel.com, haijun.liu@...iatek.com,
	m.chetan.kumar@...ux.intel.com, ricardo.martinez@...ux.intel.com,
	loic.poulain@...aro.org, ryazanov.s.a@...il.com,
	johannes@...solutions.net, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, linux-kernel@...r.kernel.com,
	vsankar@...ovo.com, danielwinkler@...gle.com, nmarupaka@...gle.com,
	joey.zhao@...ocom.com, liuqf@...ocom.com, felix.yan@...ocom.com,
	Jinjian Song <jinjian.song@...ocom.com>
Subject: Re: [PATCH v1] net: wwan: t7xx: Add fastboot interface

On Thu, Dec 21, 2023 at 03:09:51PM +0800, Jinjian Song wrote:
> From: Jinjian Song <jinjian.song@...ocom.com>
> 
> To support cases such as firmware update or core dump, the t7xx
> device is capable of signaling the host that a special port needs
> to be created before the handshake phase.
> 
> Adds the infrastructure required to create the early ports which
> also requires a different configuration of CLDMA queues.
> 
> On early detection of wwan device in fastboot mode, driver sets
> up CLDMA0 HW tx/rx queues for raw data transfer and then create
> fastboot port to user space.
> 
> Signed-off-by: Jinjian Song <jinjian.song@...ocom.com>

...

> @@ -729,8 +792,17 @@ static int t7xx_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  
>  	t7xx_pcie_mac_interrupts_dis(t7xx_dev);
>  
> +	ret = sysfs_create_group(&t7xx_dev->pdev->dev.kobj,
> +				 &t7xx_mode_attribute_group);
> +	if (ret) {
> +		t7xx_md_exit(t7xx_dev);
> +		return ret;
> +	}

Hi Jinjian,

Please consider using goto labels for unwind on error in this function.
Something like this (completely untested!):

	if (ret)
		goto err_md_exit;

	...
	if (ret)
		goto err_remove_group;

	...
	return 0;

err_remove_group:
	sysfs_remove_group(&t7xx_dev->pdev->dev.kobj,
			   &t7xx_mode_attribute_group);
err_md_exit:
	t7xx_md_exit(t7xx_dev);
	return ret;

The reason that I as for this more idiomatic form is that,
in my experience, it is less error prone and easier
to maintain.

> +
>  	ret = t7xx_interrupt_init(t7xx_dev);
>  	if (ret) {
> +		sysfs_remove_group(&t7xx_dev->pdev->dev.kobj,
> +				   &t7xx_mode_attribute_group);
>  		t7xx_md_exit(t7xx_dev);
>  		return ret;
>  	}