Firmware class name collision

PATCH In review Markus Rechberger 4 participants 7 messages Aug 13, 2007 – Aug 14, 2007 Driver core

lore

Share & tools

lore

Markus Rechberger

author

2007-08-13 15:23 UTC #0 lore raw

PATCH 1/1 Firmware class name collision

Hi,

following patch fixes the i2c name collision with i2c-dev.

http://mcentral.de/wiki/index.php/Bugtracker#i2c_core_problem

This issue has been experienced with em28xx and saa7133 based devices.
I discussed that problem with Jean Delvare a while ago and he proposed to add a prefix to the class name.

http://mcentral.de/~mrec/patches/firmware_class_name_collision.diff

Signed-off-by Markus Rechberger <[email protected]>

index b24efd4..bfc54a1 100644

drivers/base/firmware_class.c +0 -0

@@ -297,8 +297,7 @@ firmware_class_timeout(u_long data)

static inline void fw_setup_device_id(struct device *f_dev, struct device *dev) {

-       /* XXX warning we should watch out for name collisions */
-       strlcpy(f_dev->bus_id, dev->bus_id, BUS_ID_SIZE);
+       snprintf(f_dev->bus_id, BUS_ID_SIZE, "fw-%s", dev->bus_id);

}

static int fw_register_device(struct device **dev_p, const char *fw_name,

Markus

Jean Delvare

Acked-by on v1 1/1

2007-08-13 15:27 UTC #1 lore raw

Hi Markus,

Quoted 11 lines · Markus Rechberger

following patch fixes the i2c name collision with i2c-dev.
http://mcentral.de/wiki/index.php/Bugtracker#i2c_core_problem
This issue has been experienced with em28xx and saa7133 based devices. I discussed that problem with Jean Delvare a while ago and he proposed to add a prefix to the class name.
http://mcentral.de/~mrec/patches/firmware_class_name_collision.diff
Signed-off-by Markus Rechberger <[email protected]>

Acked-by Jean Delvare <[email protected]>

Quoted 15 lines

index b24efd4..bfc54a1 100644

drivers/base/firmware_class.c +1 -2

@@ -297,8 +297,7 @@ firmware_class_timeout(u_long data)
 
 static inline void fw_setup_device_id(struct device *f_dev, struct 
 device *dev)
 {
-       /* XXX warning we should watch out for name collisions */
-       strlcpy(f_dev->bus_id, dev->bus_id, BUS_ID_SIZE);
+       snprintf(f_dev->bus_id, BUS_ID_SIZE, "fw-%s", dev->bus_id);
 }

static int fw_register_device(struct device **dev_p, const char *fw_name,

Signature · 1 line

Jean Delvare

Marcel Holtmann

Acked-by on v1 1/1

2007-08-13 17:19 UTC #2 lore raw

In reply to Markus Rechberger

Hi Markus,

Quoted 16 lines

following patch fixes the i2c name collision with i2c-dev.
http://mcentral.de/wiki/index.php/Bugtracker#i2c_core_problem
This issue has been experienced with em28xx and saa7133 based devices. I discussed that problem with Jean Delvare a while ago and he proposed to add a prefix to the class name.
http://mcentral.de/~mrec/patches/firmware_class_name_collision.diff
Signed-off-by Markus Rechberger <[email protected]>
index b24efd4..bfc54a1 100644

drivers/base/firmware_class.c firmware_class_timeout(u_long data) · line 302

Quoted 8 lines

drivers/base/firmware_class.c +1 -2

@@ -297,8 +297,7 @@ firmware_class_timeout(u_long data)
 
 static inline void fw_setup_device_id(struct device *f_dev, struct 
 device *dev)
 {
-       /* XXX warning we should watch out for name collisions */
-       strlcpy(f_dev->bus_id, dev->bus_id, BUS_ID_SIZE);
+       snprintf(f_dev->bus_id, BUS_ID_SIZE, "fw-%s", dev->bus_id);
 }

I would prefer if we use "firmware-%s" since the "fw" might collide with the new Firewire stack. Please change that and I agree.

Acked-by Marcel Holtmann <[email protected]>

Regards

Marcel

Markus Rechberger

author

2007-08-13 17:26 UTC #3 lore raw

Hi Marcel,

Quoted 19 lines · Marcel Holtmann

Hi Markus,
Quoted 16 lines
following patch fixes the i2c name collision with i2c-dev.
http://mcentral.de/wiki/index.php/Bugtracker#i2c_core_problem
This issue has been experienced with em28xx and saa7133 based devices.
I discussed that problem with Jean Delvare a while ago and he proposed to add a prefix to the class name.
http://mcentral.de/~mrec/patches/firmware_class_name_collision.diff
Signed-off-by Markus Rechberger <[email protected]>
index b24efd4..bfc54a1 100644
drivers/base/firmware_class.c +0 -0
@@ -297,8 +297,7 @@ firmware_class_timeout(u_long data)
 

Quoted 13 lines

drivers/base/firmware_class.c firmware_class_timeout(u_long data) · line 303
Quoted 8 lines
drivers/base/firmware_class.c +1 -2
 static inline void fw_setup_device_id(struct device *f_dev, struct 
 device *dev)
 {
-       /* XXX warning we should watch out for name collisions */
-       strlcpy(f_dev->bus_id, dev->bus_id, BUS_ID_SIZE);
+       snprintf(f_dev->bus_id, BUS_ID_SIZE, "fw-%s", dev->bus_id);
 }
 
I would prefer if we use "firmware-%s" since the "fw" might collide with the new Firewire stack. Please change that and I agree.

firmware-%s sounds more informative and cannot be mistaken with firewire yes.

Signed-off-by Markus Rechberger <[email protected]>

http://mcentral.de/~mrec/patches/firmware_class_name_collision_2.diff

drivers/base/firmware_class.c +0 -0

index b24efd4..bfc54a1 100644
@@ -297,8 +297,7 @@ firmware_class_timeout(u_long data)

static inline void fw_setup_device_id(struct device *f_dev, struct device *dev) {

-       /* XXX warning we should watch out for name collisions */
-       strlcpy(f_dev->bus_id, dev->bus_id, BUS_ID_SIZE);
+       snprintf(f_dev->bus_id, BUS_ID_SIZE, "firmware-%s", dev->bus_id);

}

static int fw_register_device(struct device **dev_p, const char *fw_name,

Quoted 10 lines

Acked-by Marcel Holtmann <[email protected]>
Regards
Marcel

Marcel Holtmann

Acked-by

2007-08-13 17:47 UTC #4 lore raw

Hi Markus,

Quoted 31 lines

Quoted 17 lines
Quoted 17 lines
following patch fixes the i2c name collision with i2c-dev.
http://mcentral.de/wiki/index.php/Bugtracker#i2c_core_problem
This issue has been experienced with em28xx and saa7133 based devices. I discussed that problem with Jean Delvare a while ago and he proposed to add a prefix to the class name.
http://mcentral.de/~mrec/patches/firmware_class_name_collision.diff
Signed-off-by Markus Rechberger <[email protected]>
index b24efd4..bfc54a1 100644
drivers/base/firmware_class.c +0 -0
@@ -297,8 +297,7 @@ firmware_class_timeout(u_long data)
 
Quoted 13 lines
drivers/base/firmware_class.c firmware_class_timeout(u_long data) · line 303
Quoted 8 lines
drivers/base/firmware_class.c +1 -2
 static inline void fw_setup_device_id(struct device *f_dev, struct 
 device *dev)
 {
-       /* XXX warning we should watch out for name collisions */
-       strlcpy(f_dev->bus_id, dev->bus_id, BUS_ID_SIZE);
+       snprintf(f_dev->bus_id, BUS_ID_SIZE, "fw-%s", dev->bus_id);
 }
 
I would prefer if we use "firmware-%s" since the "fw" might collide with the new Firewire stack. Please change that and I agree.

Quoted 4 lines

firmware-%s sounds more informative and cannot be mistaken with firewire yes.
Signed-off-by Markus Rechberger <[email protected]>

Acked-by Marcel Holtmann <[email protected]>

Regards

Marcel

Jean Delvare

2007-08-14 13:33 UTC #5 lore raw

In reply to Markus Rechberger

Hi Markus,

Quoted 16 lines · Markus Rechberger

Quoted 2 lines · Marcel Holtmann
I would prefer if we use "firmware-%s" since the "fw" might collide with the new Firewire stack. Please change that and I agree.
firmware-%s sounds more informative and cannot be mistaken with firewire yes.
Signed-off-by Markus Rechberger <[email protected]>
http://mcentral.de/~mrec/patches/firmware_class_name_collision_2.diff
drivers/base/firmware_class.c +0 -0
index b24efd4..bfc54a1 100644

drivers/base/firmware_class.c firmware_class_timeout(u_long data) · line 302

Quoted 8 lines

drivers/base/firmware_class.c +1 -2

@@ -297,8 +297,7 @@ firmware_class_timeout(u_long data)
 
 static inline void fw_setup_device_id(struct device *f_dev, struct 
 device *dev)
 {
-       /* XXX warning we should watch out for name collisions */
-       strlcpy(f_dev->bus_id, dev->bus_id, BUS_ID_SIZE);
+       snprintf(f_dev->bus_id, BUS_ID_SIZE, "firmware-%s", dev->bus_id);
 }

Please keep in mind that BUS_ID_SIZE is "only" 20. "firmware-" takes 9 characters, add one for the trailing zero and this only leaves room for 10 characters for the original bus id. While this will be enough for the i2c case, I suspect that some other bus IDs won't fit.

Signature · 1 line

Jean Delvare

Kay Sievers

2007-08-14 14:17 UTC #6 lore raw

Quoted 18 lines · Jean Delvare

Quoted 17 lines · Markus Rechberger
Quoted 2 lines · Marcel Holtmann
I would prefer if we use "firmware-%s" since the "fw" might collide with the new Firewire stack. Please change that and I agree.
firmware-%s sounds more informative and cannot be mistaken with firewire yes.
Signed-off-by Markus Rechberger <[email protected]>
http://mcentral.de/~mrec/patches/firmware_class_name_collision_2.diff
drivers/base/firmware_class.c +0 -0
index b24efd4..bfc54a1 100644
@@ -297,8 +297,7 @@ firmware_class_timeout(u_long data)
 

Quoted 12 lines

drivers/base/firmware_class.c firmware_class_timeout(u_long data) · line 302
Quoted 7 lines
drivers/base/firmware_class.c +1 -2
 static inline void fw_setup_device_id(struct device *f_dev, struct
 device *dev)
 {
-       /* XXX warning we should watch out for name collisions */
-       strlcpy(f_dev->bus_id, dev->bus_id, BUS_ID_SIZE);
+       snprintf(f_dev->bus_id, BUS_ID_SIZE, "firmware-%s", dev->bus_id);
 }
Please keep in mind that BUS_ID_SIZE is "only" 20. "firmware-" takes 9 characters, add one for the trailing zero and this only leaves room for 10 characters for the original bus id. While this will be enough for the i2c case, I suspect that some other bus IDs won't fit.

Hmm, is there any case where .bus_id is different from .kobj.k_name? We always do:
kobject_set_name(&dev->kobj, "%s", dev->bus_id);

Which looks kind of weird, as the kobject (which doesn't have that name size limitation) is always embedded in the same struct anyway.

Thanks,
Kay