drivers/acpi/scan.c:328 acpi_device_remove_files() info: redundant null check on dev

Hi Lance,

FYI, there are new smatch warnings show up in

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux.git next
head:   10eaea0474b0078bb84ff9009329b83d3e30337f
commit: 2bc8195d56163f54d3036304efa8d71ab1091bf1 [4/5] ACPI: Add new sysfs interface to export device description

+ drivers/acpi/scan.c:328 acpi_device_remove_files() info: redundant null check on dev->pnp.str_obj calling kfree()

vim +328 drivers/acpi/scan.c

e49bd2dd (Zhang Rui      2006-12-08  312) 		result = device_create_file(&dev->dev, &dev_attr_eject);
0c526d96 (Alex Chiang    2009-05-14  313) end:
e49bd2dd (Zhang Rui      2006-12-08  314) 	return result;
^1da177e (Linus Torvalds 2005-04-16  315) }
^1da177e (Linus Torvalds 2005-04-16  316) 
f883d9db (Patrick Mochel 2006-12-07  317) static void acpi_device_remove_files(struct acpi_device *dev)
^1da177e (Linus Torvalds 2005-04-16  318) {
f883d9db (Patrick Mochel 2006-12-07  319) 	acpi_status status;
f883d9db (Patrick Mochel 2006-12-07  320) 	acpi_handle temp;
^1da177e (Linus Torvalds 2005-04-16  321) 
f883d9db (Patrick Mochel 2006-12-07  322) 	/*
2bc8195d (Lance Ortiz    2012-09-07  323) 	 * If device has _STR, remove 'description' file
2bc8195d (Lance Ortiz    2012-09-07  324) 	 */
2bc8195d (Lance Ortiz    2012-09-07  325) 	status = acpi_get_handle(dev->handle, "_STR", &temp);
2bc8195d (Lance Ortiz    2012-09-07  326) 	if (ACPI_SUCCESS(status)) {
2bc8195d (Lance Ortiz    2012-09-07  327) 		if (dev->pnp.str_obj != NULL)
2bc8195d (Lance Ortiz    2012-09-07 @328) 			kfree(dev->pnp.str_obj);
2bc8195d (Lance Ortiz    2012-09-07  329) 		device_remove_file(&dev->dev, &dev_attr_description);
2bc8195d (Lance Ortiz    2012-09-07  330) 	}
2bc8195d (Lance Ortiz    2012-09-07  331) 	/*
2bc8195d (Lance Ortiz    2012-09-07  332) 	 * If device has _EJ0, remove 'eject' file.
f883d9db (Patrick Mochel 2006-12-07  333) 	 */
f883d9db (Patrick Mochel 2006-12-07  334) 	status = acpi_get_handle(dev->handle, "_EJ0", &temp);
f883d9db (Patrick Mochel 2006-12-07  335) 	if (ACPI_SUCCESS(status))
f883d9db (Patrick Mochel 2006-12-07  336) 		device_remove_file(&dev->dev, &dev_attr_eject);

---
0-DAY kernel build testing backend Open Source Technology Centre
Fengguang Wu, Yuanhan Liu Intel Corporation

Thanks for letting me know. I send out an updated patch removing the redundant check. But I am wondering if we should leave it in for safety's sake. Is having a info message like this from smatch a problem?

Lance

-----Original Message-----
From: Fengguang Wu [mailto:[email protected]]
Sent: Tuesday, October 02, 2012 1:40 AM
To: Ortiz, Lance E
Cc: [email protected]; Len Brown; [email protected]; [email protected]
Subject: [acpi:next 4/5] drivers/acpi/scan.c:328 acpi_device_remove_files() info: redundant null check on dev->pnp.str_obj calling kfree()

Hi Lance,

FYI, there are new smatch warnings show up in

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux.git next
head:   10eaea0474b0078bb84ff9009329b83d3e30337f
commit: 2bc8195d56163f54d3036304efa8d71ab1091bf1 [4/5] ACPI: Add new sysfs interface to export device description

+ drivers/acpi/scan.c:328 acpi_device_remove_files() info: redundant null check on dev->pnp.str_obj calling kfree()

vim +328 drivers/acpi/scan.c

e49bd2dd (Zhang Rui      2006-12-08  312) 		result = device_create_file(&dev->dev, &dev_attr_eject);
0c526d96 (Alex Chiang    2009-05-14  313) end:
e49bd2dd (Zhang Rui      2006-12-08  314) 	return result;
^1da177e (Linus Torvalds 2005-04-16  315) }
^1da177e (Linus Torvalds 2005-04-16  316) 
f883d9db (Patrick Mochel 2006-12-07  317) static void acpi_device_remove_files(struct acpi_device *dev)
^1da177e (Linus Torvalds 2005-04-16  318) {
f883d9db (Patrick Mochel 2006-12-07  319) 	acpi_status status;
f883d9db (Patrick Mochel 2006-12-07  320) 	acpi_handle temp;
^1da177e (Linus Torvalds 2005-04-16  321) 
f883d9db (Patrick Mochel 2006-12-07  322) 	/*
2bc8195d (Lance Ortiz    2012-09-07  323) 	 * If device has _STR, remove 'description' file
2bc8195d (Lance Ortiz    2012-09-07  324) 	 */
2bc8195d (Lance Ortiz    2012-09-07  325) 	status = acpi_get_handle(dev->handle, "_STR", &temp);
2bc8195d (Lance Ortiz    2012-09-07  326) 	if (ACPI_SUCCESS(status)) {
2bc8195d (Lance Ortiz    2012-09-07  327) 		if (dev->pnp.str_obj != NULL)
2bc8195d (Lance Ortiz    2012-09-07 @328) 			kfree(dev->pnp.str_obj);
2bc8195d (Lance Ortiz    2012-09-07  329) 		device_remove_file(&dev->dev, &dev_attr_description);
2bc8195d (Lance Ortiz    2012-09-07  330) 	}
2bc8195d (Lance Ortiz    2012-09-07  331) 	/*
2bc8195d (Lance Ortiz    2012-09-07  332) 	 * If device has _EJ0, remove 'eject' file.
f883d9db (Patrick Mochel 2006-12-07  333) 	 */
f883d9db (Patrick Mochel 2006-12-07  334) 	status = acpi_get_handle(dev->handle, "_EJ0", &temp);
f883d9db (Patrick Mochel 2006-12-07  335) 	if (ACPI_SUCCESS(status))
f883d9db (Patrick Mochel 2006-12-07  336) 		device_remove_file(&dev->dev, &dev_attr_eject);

---
0-DAY kernel build testing backend Open Source Technology Centre
Fengguang Wu, Yuanhan Liu Intel Corporation

There is no safety issue here. kfree() will always accept NULL pointers. It's cleaner to leave it out.

It's not a problem. We just send you the one email is all.

regards,
dan carpenter