Skip to contentSkip to conversation

POWERPC: don't cast a pointer to pointer of list_head

PATCH In review Li Zefan 2 participants 3 messages Dec 6, 2007 – Dec 7, 2007

lore
Share & tools
lore

Li Zefan

author
#0 lore raw

PATCH 1/3 POWERPC: don't cast a pointer to pointer of list_head

The casting is safe only when the list_head member is the first member of the structure.

---
 arch/ppc/syslib/ocp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/ppc/syslib/ocp.c +1 -1
index 3f5be2c..d42d408 100644
@@ -376,7 +376,7 @@ ocp_remove_one_device(unsigned int vendor, unsigned int function, int index)
 
 	down_write(&ocp_devices_sem);
 	dev = __ocp_find_device(vendor, function, index);
-	list_del((struct list_head *)dev);
+	list_del(&dev->link);
 	up_write(&ocp_devices_sem);
 
 	DBG(("ocp: ocp_remove_one_device(vendor: %x, function: %x, index: %d)... done.\n", vendor, function, index));
Signature · 1 line
1.5.3.rc7
#1 lore raw
Quoted 2 lines · Li Zefan

The casting is safe only when the list_head member is the first member of the structure.

Even so, I don't think too safe :) It might technically work, but it could break more easily.

So even if you find places where list_head is the first member of a structure, it would be nice to explicitly use the list_head member and avoid casts IMO.

Thanks,
Nick

Quoted 21 lines
---
 arch/ppc/syslib/ocp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/ppc/syslib/ocp.c +1 -1
index 3f5be2c..d42d408 100644
@@ -376,7 +376,7 @@ ocp_remove_one_device(unsigned int vendor, unsigned int
 function, int index)
 
 	down_write(&ocp_devices_sem);
 	dev = __ocp_find_device(vendor, function, index);
-	list_del((struct list_head *)dev);
+	list_del(&dev->link);
 	up_write(&ocp_devices_sem);
 

DBG(("ocp: ocp_remove_one_device(vendor: %x, function: %x, index: %d)...
done.\n", vendor, function, index));

author
#2 lore raw
Quoted 11 lines · Nick Piggin
Quoted 2 lines · Li Zefan

The casting is safe only when the list_head member is the first member of the structure.

Even so, I don't think too safe :) It might technically work, but it could break more easily.

So even if you find places where list_head is the first member of a structure, it would be nice to explicitly use the list_head member and avoid casts IMO.

This is exactly what I think. Those patches actually fix no bugs, but avoid this kind of technically and currently correct casting.

Quoted 24 lines
Quoted 22 lines
---
 arch/ppc/syslib/ocp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/ppc/syslib/ocp.c +1 -1
index 3f5be2c..d42d408 100644
@@ -376,7 +376,7 @@ ocp_remove_one_device(unsigned int vendor, unsigned int
 function, int index)
 
 	down_write(&ocp_devices_sem);
 	dev = __ocp_find_device(vendor, function, index);
-	list_del((struct list_head *)dev);
+	list_del(&dev->link);
 	up_write(&ocp_devices_sem);
 

DBG(("ocp: ocp_remove_one_device(vendor: %x, function: %x, index: %d)...
done.\n", vendor, function, index));