linux-kernel - Re: [PATCH 3/5]acpi:glue.c Fix warning: variable 'ret' set but not used

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]

Date:	Wed, 30 Jun 2010 10:13:35 +0100
From:	David Howells <dhowells@...hat.com>
To:	"Justin P. Mattock" <justinmattock@...il.com>
Cc:	dhowells@...hat.com, linux-kernel@...r.kernel.org,
	linux-acpi@...r.kernel.org, lenb@...nel.org
Subject: Re: [PATCH 3/5]acpi:glue.c Fix warning: variable 'ret' set but not used

Justin P. Mattock <justinmattock@...il.com> wrote:

>  	if (!ACPI_FAILURE(status)) {
> -		int ret;
> 
> -		ret = sysfs_create_link(&dev->kobj, &acpi_dev->dev.kobj,
> +		fn = sysfs_create_link(&dev->kobj, &acpi_dev->dev.kobj,
>  				"firmware_node");
> -		ret = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
> +		pn = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
>  				"physical_node");
> +	if (fn) {

That new if-statement still needs indenting one more tab stop.  It's indented
the same as the previous if-statement, but is actually in the body of that
previous if-statement.

The body of the second if-statement should be indented one tab beyond the if,
and else/else-if statements and the final closing brace should be indented
level with the if:

	if (...) {
		body;
	} else if (...) {
		body;
	} else {
		body;
	}

so that they line up vertically.

> +		dev_warn(dev, "dev:%p Failed to create firmware_node: %d\n",
> +			acpi_dev, fn);

The "dev:%p " seems like it ought to be superfluous if you're using
dev_warn(), and certainly, returning the pointer isn't really useful, I
suspect.

However, at this point you have two device struct pointers: dev and
&acip_dev->dev, so printing them both is may be good.  Perhaps something like:

+		dev_warn(&acpi_dev->dev,
+			 "Failed to create firmware_node link to %s %s: %d\n",
+			 dev_driver_string(dev), dev_name(dev), fn);

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/