RE: [Bug-fix]:2.6.25-rc0 Generic thermal management [Patch 1/2]: validating input parameters

[Thomas, Sujith]

> On Mon, 11 Feb 2008 15:57:06 +0530
> "Thomas, Sujith" <sujith.thomas@xxxxxxxxx> wrote:

> > From: Thomas Sujith <sujith.thomas@xxxxxxxxx>
> > 
> > Added sanity checks for interface functions in thermal with
> > other modules such as fan, processor, video etc..
> > 
> > Signed-off-by: Thomas Sujith <sujith.thomas@xxxxxxxxx>
> > ---
> > drivers/thermal/thermal.c |   69
> > +++++++++++++++++++++++++++++-----------------
> 
> The patch is fairly seriously wordwrapped.
Should be fixed now.
> 
> >  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;
> 
> 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
> hide that sloppiness.
I agree; removed
> 
> > -		return NULL;
> > +		return  ERR_PTR(-EINVAL);
> 
> the patch adds several spaces like this in places where we don't
normally
> put them.
I agree; removed


From: Thomas Sujith <sujith.thomas@xxxxxxxxx>

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@xxxxxxxxx>
---
 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 *thermal_c
 	if (result) {
 		release_idr(&thermal_cdev_idr, &thermal_idr_lock,
cdev->id);
 		kfree(cdev);
-		return NULL;
+		return ERR_PTR(result);
 	}
 
 	/* sys I/F */
-	if (type) {
-		result = device_create_file(&cdev->device,
-					    &dev_attr_cdev_type);
-		if (result)
-			goto unregister;
-	}
+	result = device_create_file(&cdev->device,
+				    &dev_attr_cdev_type);
+	if (result)
+		goto unregister;
 
 	result = device_create_file(&cdev->device, &dev_attr_max_state);
 	if (result)
@@ -490,7 +502,7 @@ struct thermal_cooling_device *thermal_c
       unregister:
 	release_idr(&thermal_cdev_idr, &thermal_idr_lock, cdev->id);
 	device_unregister(&cdev->device);
-	return NULL;
+	return ERR_PTR(result);
 }
 EXPORT_SYMBOL(thermal_cooling_device_register);
 
@@ -559,18 +571,21 @@ struct thermal_zone_device *thermal_zone
 	int result;
 	int count;
 
+	if (!type)
+		return ERR_PTR(-EINVAL);
+
 	if (strlen(type) >= THERMAL_NAME_LENGTH)
-		return NULL;
+		return ERR_PTR(-EINVAL);
 
 	if (trips > THERMAL_MAX_TRIPS || trips < 0)
-		return NULL;
+		return ERR_PTR(-EINVAL);
 
 	if (!ops || !ops->get_temp)
-		return NULL;
+		return ERR_PTR(-EINVAL);
 
 	tz = kzalloc(sizeof(struct thermal_zone_device), GFP_KERNEL);
 	if (!tz)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	INIT_LIST_HEAD(&tz->cooling_devices);
 	idr_init(&tz->idr);
@@ -578,7 +593,7 @@ struct thermal_zone_device *thermal_zone
 	result = get_idr(&thermal_tz_idr, &thermal_idr_lock, &tz->id);
 	if (result) {
 		kfree(tz);
-		return NULL;
+		return ERR_PTR(result);
 	}
 
 	strcpy(tz->type, type);
@@ -591,15 +606,13 @@ struct thermal_zone_device *thermal_zone
 	if (result) {
 		release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
 		kfree(tz);
-		return NULL;
+		return ERR_PTR(result);
 	}
 
 	/* sys I/F */
-	if (type) {
-		result = device_create_file(&tz->device,
&dev_attr_type);
-		if (result)
-			goto unregister;
-	}
+	result = device_create_file(&tz->device, &dev_attr_type);
+	if (result)
+		goto unregister;
 
 	result = device_create_file(&tz->device, &dev_attr_temp);
 	if (result)
@@ -633,7 +646,7 @@ struct thermal_zone_device *thermal_zone
       unregister:
 	release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
 	device_unregister(&tz->device);
-	return NULL;
+	return ERR_PTR(result);
 }
 EXPORT_SYMBOL(thermal_zone_device_register);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/