Skip to contentSkip to conversation

power: battery: pointer math issue in gab_probe()

PATCH Applied Dan Carpenter 3 participants 7 messages Sep 29, 2012 – Nov 17, 2012 Kernel janitors

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 power: battery: pointer math issue in gab_probe()

psy->properties is an enum (32 bit type) so adding sizeof() puts us four times further along than we intended. It should be cast to a char pointer before doing the math.

---
Casting to void * would also work on GCC, at least.

drivers/power/generic-adc-battery.c +2 -1
index 9bdf444..776f118 100644
@@ -279,7 +279,8 @@ static int __devinit gab_probe(struct platform_device *pdev)
 	}
 
 	memcpy(psy->properties, gab_props, sizeof(gab_props));
-	properties = psy->properties + sizeof(gab_props);
+	properties = (enum power_supply_property *)
+				((char *)psy->properties + sizeof(gab_props));
 
 	/*
 	 * getting channel from iio and copying the battery properties
#1 lore raw
Quoted 3 lines · Dan Carpenter

psy->properties is an enum (32 bit type) so adding sizeof() puts us four times further along than we intended. It should be cast to a char pointer before doing the math.

You really read this driver to find out this one.
Good one.

Quoted 19 lines

---
Casting to void * would also work on GCC, at least.

drivers/power/generic-adc-battery.c +2 -1
index 9bdf444..776f118 100644
@@ -279,7 +279,8 @@ static int __devinit gab_probe(struct platform_device *pdev)
 	}
 
 	memcpy(psy->properties, gab_props, sizeof(gab_props));
-	properties = psy->properties + sizeof(gab_props);
+	properties = (enum power_supply_property *)
+				((char *)psy->properties + sizeof(gab_props));
 
 	/*
 	 * getting channel from iio and copying the battery properties
#2 lore raw
Quoted 6 lines · anish kumar
Quoted 3 lines · Dan Carpenter

psy->properties is an enum (32 bit type) so adding sizeof() puts us four times further along than we intended. It should be cast to a char pointer before doing the math.

You really read this driver to find out this one.
Good one.

Nah. It's a static checker thing with Smatch
(http://smatch.sf.net). When you add pointer + sizeof(), the pointer should always be char.

regards,
dan carpenter

#3 lore raw
In reply to anish kumar

Hello Dan,

Is this patch of yours picked up by anyone?

Quoted 26 lines · anish kumar
Quoted 3 lines · Dan Carpenter

psy->properties is an enum (32 bit type) so adding sizeof() puts us four times further along than we intended. It should be cast to a char pointer before doing the math.

You really read this driver to find out this one.
Good one.

Quoted 19 lines

---
Casting to void * would also work on GCC, at least.

drivers/power/generic-adc-battery.c +2 -1
index 9bdf444..776f118 100644
@@ -279,7 +279,8 @@ static int __devinit gab_probe(struct platform_device *pdev)
 	}
 
 	memcpy(psy->properties, gab_props, sizeof(gab_props));
-	properties = psy->properties + sizeof(gab_props);
+	properties = (enum power_supply_property *)
+				((char *)psy->properties + sizeof(gab_props));
 
 	/*
 	 * getting channel from iio and copying the battery properties
#4 lore raw
Quoted 3 lines · anish kumar

Hello Dan,

Is this patch of yours picked up by anyone?

David this should go through you?

regards,
dan carpenter

#5 lore raw
Quoted 6 lines · Dan Carpenter
Quoted 3 lines · anish kumar

Hello Dan,

Is this patch of yours picked up by anyone?

David this should go through you?

I'll merge it soon. Sorry for the delay, quite busy lately. :-/

Thanks!
Anton.

#6 lore raw
In reply to Dan Carpenter
Quoted 7 lines · Dan Carpenter

psy->properties is an enum (32 bit type) so adding sizeof() puts us four times further along than we intended. It should be cast to a char pointer before doing the math.

---
Casting to void * would also work on GCC, at least.

Applied, thanks a lot!

Quoted 15 lines
drivers/power/generic-adc-battery.c +2 -1
index 9bdf444..776f118 100644
@@ -279,7 +279,8 @@ static int __devinit gab_probe(struct platform_device *pdev)
 	}
 
 	memcpy(psy->properties, gab_props, sizeof(gab_props));
-	properties = psy->properties + sizeof(gab_props);
+	properties = (enum power_supply_property *)
+				((char *)psy->properties + sizeof(gab_props));
 
 	/*
 	 * getting channel from iio and copying the battery properties