From: Thomas Sujith <sujith.thomas@intel.com>

Added sanity checks for interface functions in thermal with
other modules such as fan, processor, video etc..

Signed-off-by: Thomas Sujith <sujith.thomas@intel.com>
---
drivers/thermal/thermal.c |   69
+++++++++++++++++++++++++++++-----------------
 1 files changed, 44 insertions(+), 25 deletions(-)

Index: linux-2.6.24-rc3/drivers/thermal/thermal.c
===================================================================
--- linux-2.6.24-rc3.orig/drivers/thermal/thermal.c
+++ linux-2.6.24-rc3/drivers/thermal/thermal.c
@@ -301,13 +301,27 @@ int thermal_zone_bind_cooling_device(str
 {
 	struct thermal_cooling_device_instance *dev;
 	struct thermal_cooling_device_instance *pos;
+	struct thermal_zone_device *pos1;
+	struct thermal_cooling_device *pos2;
 	int result;
 
+	if (!tz || !cdev)
+		return -EINVAL;
+
 	if (trip >= tz->trips ||
 	    (trip < 0 && trip != THERMAL_TRIPS_NONE))
 		return -EINVAL;
 
-	if (!tz || !cdev)
+	list_for_each_entry(pos1, &thermal_tz_list, node) {
+		if (pos1 == tz)
+			break;
+	}
+	list_for_each_entry(pos2, &thermal_cdev_list, node) {
+		if (pos2 == cdev)
+			break;
+	}
+
+	if (tz != pos1 || cdev != pos2)
 		return -EINVAL;
 
 	dev =
@@ -373,6 +387,9 @@ int thermal_zone_unbind_cooling_device(s
 {
 	struct thermal_cooling_device_instance *pos, *next;
 
+	if (!tz || !cdev)
+		return -EINVAL;
+
 	mutex_lock(&tz->lock);
 	list_for_each_entry_safe(pos, next, &tz->cooling_devices, node)
{
 		if (pos->tz == tz && pos->trip == trip
@@ -427,21 +444,24 @@ struct thermal_cooling_device *thermal_c
 	struct thermal_zone_device *pos;
 	int result;
 
+	if (!type)
+		return ERR_PTR(-EINVAL);
+
 	if (strlen(type) >= THERMAL_NAME_LENGTH)
-		return NULL;
+		return ERR_PTR(-EINVAL);
 
 	if (!ops || !ops->get_max_state || !ops->get_cur_state ||
 		!ops->set_cur_state)
-		return NULL;
+		return ERR_PTR(-EINVAL);
 
 	cdev = kzalloc(sizeof(struct thermal_cooling_device),
GFP_KERNEL);
 	if ...
[ message continues ]

On Mon, 11 Feb 2008 15:57:06 +0530


Is this change actually needed?  It's sloppy for a caller to be passing
invalid arguments into a callee, and it's not good for the callee to just

the patch adds several spaces like this in places where we don't normally
put them.


--

[ message continues ]

I agree; removed


From: Thomas Sujith <sujith.thomas@intel.com>

Added sanity checks for interface functions in thermal with
other modules such as fan, processor, video etc. Using ERR_PTR
to return errors.
 
Signed-off-by: Thomas Sujith <sujith.thomas@intel.com>
---
 drivers/thermal/thermal.c |   63
+++++++++++++++++++++++++++-------------------
 1 files changed, 38 insertions(+), 25 deletions(-)

Index: linux-2.6.24-rc3/drivers/thermal/thermal.c
===================================================================
--- linux-2.6.24-rc3.orig/drivers/thermal/thermal.c
+++ linux-2.6.24-rc3/drivers/thermal/thermal.c
@@ -301,13 +301,24 @@ int thermal_zone_bind_cooling_device(str
 {
 	struct thermal_cooling_device_instance *dev;
 	struct thermal_cooling_device_instance *pos;
+	struct thermal_zone_device *pos1;
+	struct thermal_cooling_device *pos2;
 	int result;
 
 	if (trip >= tz->trips ||
 	    (trip < 0 && trip != THERMAL_TRIPS_NONE))
 		return -EINVAL;
 
-	if (!tz || !cdev)
+	list_for_each_entry(pos1, &thermal_tz_list, node) {
+		if (pos1 == tz)
+			break;
+	}
+	list_for_each_entry(pos2, &thermal_cdev_list, node) {
+		if (pos2 == cdev)
+			break;
+	}
+
+	if (tz != pos1 || cdev != pos2)
 		return -EINVAL;
 
 	dev =
@@ -427,21 +438,24 @@ struct thermal_cooling_device *thermal_c
 	struct thermal_zone_device *pos;
 	int result;
 
+	if (!type)
+		return ERR_PTR(-EINVAL);
+
 	if (strlen(type) >= THERMAL_NAME_LENGTH)
-		return NULL;
+		return ERR_PTR(-EINVAL);
 
 	if (!ops || !ops->get_max_state || !ops->get_cur_state ||
 		!ops->set_cur_state)
-		return NULL;
+		return ERR_PTR(-EINVAL);
 
 	cdev = kzalloc(sizeof(struct thermal_cooling_device),
GFP_KERNEL);
 	if (!cdev)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	result = get_idr(&thermal_cdev_idr, &thermal_idr_lock,
&cdev->id);
 	if (result) {
 		kfree(cdev);
-		return NULL;
+		return ERR_PTR(result);
 	}
 
 	strcpy(cdev->type, type);
@@ -453,16 +467,14 @@ struct thermal_cooling_device ...
[ message continues ]

On Tue, 12 Feb 2008 21:27:44 +0530

No, even after I fixed all the wordwrapping I saw a large amount
of fuzz and several rejects when trying to apply the patch.

There's a reason why Len uses his kernel.org account to get real work done ;)
--

[ message continues ]

=20
Hi Andrew,
	For time being I am attaching the patch and in the meanwhile
I'll figure out
a way to fix the wordwrap issues. This was what Len Brown also had
recommended.

Regards,
[ message continues ]

On Wed, 13 Feb 2008 16:33:10 +0530

This patch has no changelog.  Please include the full and up-to-date
changelog with each iteration of a patch.

This patch doesn't apply.  My version of Linux has

	if (trip >= tz->trips || (trip < 0 && trip != THERMAL_TRIPS_NONE))
		return -EINVAL;

whereas yours apparently has

  	if (trip >= tz->trips ||
  	    (trip < 0 && trip != THERMAL_TRIPS_NONE))
  		return -EINVAL;

(plus other changes).

Maybe you have some other patch which predeced this one.

You apparently didn't resend "[Bug-fix]:2.6.25-rc0 Generic thermal
management:ACPI [Patch 2/2]: buildfix for CONFIG_THERMAL=n" which was also
mangled.


So please just start again.  Resend all patches, against latest mainline,
with full changelogs and appropriate cc's, in an unmangled form.

Also it would be good if you could be a bit more conventional with the
patch titling.  For this one, 

Subject: [patch 1/2] Generic thermal management: validate input parameters

would suit.  Section 14 of Documentation/SubmittingPatches has some
explanation.

Thanks.
--

[ message continues ]