Skip to contentSkip to conversation

Misc driver core changes (constness)

PATCH In review Dmitry Torokhov 2 participants 8 messages Apr 26, 2005 – Apr 29, 2005

Subsystems (5)
USB PCI Library code InfiniBand +1 more subsystems
lore
Share & tools
lore

Dmitry Torokhov

author
#0 lore raw

cover Misc driver core changes (constness)

Hi,

It all started when code like this:

static const char driver_name = "blah";
static struct device_driver {
.name = driver_name,
};

would give me compiler warning about removing constness because driver core has "name" fields drclared simply as "char *". I think it is a good idea to have them as "const char *" since whoever accesses them should not try to change them.

01-hotplug-use-kobject-name.patch
- kobject_hotplug should use kobject_name() instead of accessing kobj->name directly since for objects with long names it can contain garbage.

02-sysfs-link-constness.patch
- make sysfs_{create|remove}_link to take const char * name.

03-kobject-const-name.patch
- make kobject's name const char * since users should not attempt to change it (except by calling kobject_rename).

04-kset-name-const.patch
- change name() method in kset_hiotplug_ops return const char * since users shoudl not try to modify returned data.

05-driver-const-name.patch
- change driver's, bus's, class's and platform device's names to be const char * so one can use const char *drv_name = "asdfg"; when initializing structures.
Also kill couple of whitespaces.

Please consider for inclusion.

Signature · 1 line
Dmitry
author
#1 lore raw

PATCH 1/5 kobject_hotplug() should use kobject_name()

kobject: kobject_hotplug should use kobject_name() instead of
accessing kobj->name directly since for objects with long names it can contain garbage.

Signed-off-by Dmitry Torokhov <[email protected]>

---

 kobject_uevent.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
lib/kobject_uevent.c +2 -2
Index: dtor/lib/kobject_uevent.c
===================================================================
@@ -246,10 +246,10 @@ void kobject_hotplug(struct kobject *kob
 	if (hotplug_ops->name)
 		name = hotplug_ops->name(kset, kobj);
 	if (name == NULL)
-		name = kset->kobj.name;
+		name = kobject_name(&kset->kobj);
 
 	argv [0] = hotplug_path;
-	argv [1] = name;
+	argv [1] = name;
 	argv [2] = NULL;
 
 	/* minimal command environment */
author
#2 lore raw

PATCH 2/5 sysfs_{create|remove}_link should take const char *

In reply to Dmitry Torokhov

sysfs: make sysfs_{create|remove}_link to take const char * name.

Signed-off-by Dmitry Torokhov <[email protected]>

---

 fs/sysfs/symlink.c    |    8 ++++----
 include/linux/sysfs.h |   10 +++++-----
 2 files changed, 9 insertions(+), 9 deletions(-)
include/linux/sysfs.h +5 -5
Index: dtor/include/linux/sysfs.h
===================================================================
@@ -105,11 +105,11 @@ sysfs_chmod_file(struct kobject *kobj, s
 extern void
 sysfs_remove_file(struct kobject *, const struct attribute *);
 
-extern int 
-sysfs_create_link(struct kobject * kobj, struct kobject * target, char * name);
+extern int
+sysfs_create_link(struct kobject * kobj, struct kobject * target, const char * name);
 
 extern void
-sysfs_remove_link(struct kobject *, char * name);
+sysfs_remove_link(struct kobject *, const char * name);
 
 int sysfs_create_bin_file(struct kobject * kobj, struct bin_attribute * attr);
 int sysfs_remove_bin_file(struct kobject * kobj, struct bin_attribute * attr);

@@ -153,12 +153,12 @@ static inline void sysfs_remove_file(str
 	;
 }
 
-static inline int sysfs_create_link(struct kobject * k, struct kobject * t, char * n)
+static inline int sysfs_create_link(struct kobject * k, struct kobject * t, const char * n)
 {
 	return 0;
 }
 
-static inline void sysfs_remove_link(struct kobject * k, char * name)
+static inline void sysfs_remove_link(struct kobject * k, const char * name)
 {
 	;
 }
fs/sysfs/symlink.c +4 -4
Index: dtor/fs/sysfs/symlink.c
===================================================================
@@ -43,7 +43,7 @@ static void fill_object_path(struct kobj
 	}
 }
 
-static int sysfs_add_link(struct dentry * parent, char * name, struct kobject * target)
+static int sysfs_add_link(struct dentry * parent, const char * name, struct kobject * target)
 {
 	struct sysfs_dirent * parent_sd = parent->d_fsdata;
 	struct sysfs_symlink * sl;

@@ -79,7 +79,7 @@ exit1:
  *	@target:	object we're pointing to.
  *	@name:		name of the symlink.
  */
-int sysfs_create_link(struct kobject * kobj, struct kobject * target, char * name)
+int sysfs_create_link(struct kobject * kobj, struct kobject * target, const char * name)
 {
 	struct dentry * dentry = kobj->dentry;
 	int error = 0;

@@ -99,13 +99,13 @@ int sysfs_create_link(struct kobject * k
  *	@name:	name of the symlink to remove.
  */
 
-void sysfs_remove_link(struct kobject * kobj, char * name)
+void sysfs_remove_link(struct kobject * kobj, const char * name)
 {
 	sysfs_hash_and_remove(kobj->dentry,name);
 }
 
 static int sysfs_get_target_path(struct kobject * kobj, struct kobject * target,
-				   char *path)
+				 char *path)
 {
 	char * s;
 	int depth, size;
author
#3 lore raw

PATCH 3/5 Make kobject's name be const char *

In reply to Dmitry Torokhov

kobject: make kobject's name const char * since users should not
attempt to change it (except by calling kobject_rename).

Signed-off-by Dmitry Torokhov <[email protected]>

---

 include/linux/kobject.h |    6 +++---
 lib/kobject.c           |    2 +-
 lib/kobject_uevent.c    |    4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)
include/linux/kobject.h +3 -3
Index: dtor/include/linux/kobject.h
===================================================================
@@ -33,7 +33,7 @@
 extern u64 hotplug_seqnum;
 
 struct kobject {
-	char			* k_name;
+	const char		* k_name;
 	char			name[KOBJ_NAME_LEN];
 	struct kref		kref;
 	struct list_head	entry;

@@ -46,7 +46,7 @@ struct kobject {
 extern int kobject_set_name(struct kobject *, const char *, ...)
 	__attribute__((format(printf,2,3)));
 
-static inline char * kobject_name(struct kobject * kobj)
+static inline const char * kobject_name(const struct kobject * kobj)
 {
 	return kobj->k_name;
 }

@@ -57,7 +57,7 @@ extern void kobject_cleanup(struct kobje
 extern int kobject_add(struct kobject *);
 extern void kobject_del(struct kobject *);
 
-extern int kobject_rename(struct kobject *, char *new_name);
+extern int kobject_rename(struct kobject *, const char *new_name);
 
 extern int kobject_register(struct kobject *);
 extern void kobject_unregister(struct kobject *);
lib/kobject.c +1 -1
Index: dtor/lib/kobject.c
===================================================================
@@ -280,7 +280,7 @@ EXPORT_SYMBOL(kobject_set_name);
  *	@new_name: object's new name
  */
 
-int kobject_rename(struct kobject * kobj, char *new_name)
+int kobject_rename(struct kobject * kobj, const char *new_name)
 {
 	int error = 0;
lib/kobject_uevent.c +2 -2
Index: dtor/lib/kobject_uevent.c
===================================================================
@@ -197,7 +197,7 @@ void kobject_hotplug(struct kobject *kob
 	int i = 0;
 	int retval;
 	char *kobj_path = NULL;
-	char *name = NULL;
+	const char *name = NULL;
 	char *action_string;
 	u64 seq;
 	struct kobject *top_kobj = kobj;

@@ -249,7 +249,7 @@ void kobject_hotplug(struct kobject *kob
 		name = kobject_name(&kset->kobj);
 
 	argv [0] = hotplug_path;
-	argv [1] = name;
+	argv [1] = (char *)name; /* won't be changed but 'const' has to go */
 	argv [2] = NULL;
 
 	/* minimal command environment */
author
#4 lore raw

PATCH 4/5 kset_hotplug_ops->name shoudl return const char *

In reply to Dmitry Torokhov

kobject: change name() method in kset_hotplug_ops return const char *
since users shoudl not try to modify returned data.

Signed-off-by Dmitry Torokhov <[email protected]>
---
 drivers/base/class.c    |    2 +-
 drivers/base/core.c     |    2 +-
 include/linux/kobject.h |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
drivers/base/class.c +1 -1
Index: dtor/drivers/base/class.c
===================================================================
@@ -262,7 +262,7 @@ static int class_hotplug_filter(struct k
 	return 0;
 }
 
-static char *class_hotplug_name(struct kset *kset, struct kobject *kobj)
+static const char *class_hotplug_name(struct kset *kset, struct kobject *kobj)
 {
 	struct class_device *class_dev = to_class_dev(kobj);
include/linux/kobject.h +1 -1
Index: dtor/include/linux/kobject.h
===================================================================
@@ -94,7 +94,7 @@ struct kobj_type {
  */
 struct kset_hotplug_ops {
 	int (*filter)(struct kset *kset, struct kobject *kobj);
-	char *(*name)(struct kset *kset, struct kobject *kobj);
+	const char *(*name)(struct kset *kset, struct kobject *kobj);
 	int (*hotplug)(struct kset *kset, struct kobject *kobj, char **envp,
 			int num_envp, char *buffer, int buffer_size);
 };
drivers/base/core.c +1 -1
Index: dtor/drivers/base/core.c
===================================================================
@@ -105,7 +105,7 @@ static int dev_hotplug_filter(struct kse
 	return 0;
 }
 
-static char *dev_hotplug_name(struct kset *kset, struct kobject *kobj)
+static const char *dev_hotplug_name(struct kset *kset, struct kobject *kobj)
 {
 	struct device *dev = to_dev(kobj);
author
#5 lore raw

PATCH 5/5 make driver's name be const char *

In reply to Dmitry Torokhov

Driver core:
change driver's, bus's, class's and platform device's names to be const char * so one can use

            const char *drv_name = "asdfg";

when initializing structures.
Also kill couple of whitespaces.

Signed-off-by Dmitry Torokhov <[email protected]>

---

 drivers/usb/core/devices.c |    2 +-
 include/linux/device.h     |   12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)
include/linux/device.h +6 -6
Index: dtor/include/linux/device.h
===================================================================
@@ -47,7 +47,7 @@ struct class_device;
 struct class_simple;
 
 struct bus_type {
-	char			* name;
+	const char		* name;
 
 	struct subsystem	subsys;
 	struct kset		drivers;

@@ -98,17 +98,17 @@ extern int bus_create_file(struct bus_ty
 extern void bus_remove_file(struct bus_type *, struct bus_attribute *);
 
 struct device_driver {
-	char			* name;
+	const char		* name;
 	struct bus_type		* bus;
 
 	struct completion	unloaded;
 	struct kobject		kobj;
 	struct list_head	devices;
 
-	struct module 		* owner;
+	struct module		* owner;
 
 	int	(*probe)	(struct device * dev);
-	int 	(*remove)	(struct device * dev);
+	int	(*remove)	(struct device * dev);
 	void	(*shutdown)	(struct device * dev);
 	int	(*suspend)	(struct device * dev, pm_message_t state, u32 level);
 	int	(*resume)	(struct device * dev, u32 level);

@@ -142,7 +142,7 @@ extern void driver_remove_file(struct de
  * device classes
  */
 struct class {
-	char			* name;
+	const char		* name;
 
 	struct subsystem	subsys;
 	struct list_head	children;

@@ -369,7 +369,7 @@ extern struct device *device_find(const 
 /* drivers/base/platform.c */
 
 struct platform_device {
-	char		* name;
+	const char	* name;
 	u32		id;
 	struct device	dev;
 	u32		num_resources;
drivers/usb/core/devices.c +1 -1
Index: dtor/drivers/usb/core/devices.c
===================================================================
@@ -239,7 +239,7 @@ static char *usb_dump_interface_descript
 	int setno)
 {
 	const struct usb_interface_descriptor *desc = &intfc->altsetting[setno].desc;
-	char *driver_name = "";
+	const char *driver_name = "";
 
 	if (start > end)
 		return start;
#6 lore raw
In reply to Dmitry Torokhov
Quoted 30 lines · Dmitry Torokhov

Hi,

It all started when code like this:

static const char driver_name = "blah";
static struct device_driver {
.name = driver_name,
};

would give me compiler warning about removing constness because driver core has "name" fields drclared simply as "char *". I think it is a good idea to have them as "const char *" since whoever accesses them should not try to change them.

01-hotplug-use-kobject-name.patch
- kobject_hotplug should use kobject_name() instead of accessing kobj->name directly since for objects with long names it can contain garbage.

02-sysfs-link-constness.patch
- make sysfs_{create|remove}_link to take const char * name.

03-kobject-const-name.patch
- make kobject's name const char * since users should not attempt to change it (except by calling kobject_rename).

04-kset-name-const.patch
- change name() method in kset_hiotplug_ops return const char * since users shoudl not try to modify returned data.

Quoted 7 lines

05-driver-const-name.patch
- change driver's, bus's, class's and platform device's names to be const char * so one can use const char *drv_name = "asdfg"; when initializing structures.
Also kill couple of whitespaces.

Please consider for inclusion.

Very nice, I've added all 5 patches to my tree, and are queued up for after 2.6.12 is out.

thanks,

greg k-h

author
#7 lore raw

[PATCH 6/5] Make attributes names const char *

sysfs: make attributes and attribute_group's names const char *

Signed-off-by Dmitry Torokhov <[email protected]>

---

 drivers/infiniband/core/sysfs.c |  122 ++++++++++++++++------------------------
 drivers/pci/pci-sysfs.c         |   13 ++--
 include/linux/sysfs.h           |    4 -
 3 files changed, 59 insertions(+), 80 deletions(-)
include/linux/sysfs.h +2 -2
Index: dtor/include/linux/sysfs.h
===================================================================
@@ -16,13 +16,13 @@ struct kobject;
 struct module;
 
 struct attribute {
-	char			* name;
+	const char		* name;
 	struct module 		* owner;
 	mode_t			mode;
 };
 
 struct attribute_group {
-	char			* name;
+	const char		* name;
 	struct attribute	** attrs;
 };
drivers/pci/pci-sysfs.c +7 -6
Index: dtor/drivers/pci/pci-sysfs.c
===================================================================
@@ -293,16 +293,17 @@ pci_create_resource_files(struct pci_dev
 		if (!pci_resource_len(pdev, i))
 			continue;
 
-		res_attr = kmalloc(sizeof(*res_attr) + 10, GFP_ATOMIC);
+		/* allocate attribute structure, piggyback attribute name */
+		res_attr = kcalloc(1, sizeof(*res_attr) + 10, GFP_ATOMIC);
 		if (res_attr) {
-			memset(res_attr, 0, sizeof(*res_attr) + 10);
+			char *res_attr_name = (char *)(res_attr + 1);
+
 			pdev->res_attr[i] = res_attr;
-			/* Allocated above after the res_attr struct */
-			res_attr->attr.name = (char *)(res_attr + 1);
-			sprintf(res_attr->attr.name, "resource%d", i);
-			res_attr->size = pci_resource_len(pdev, i);
+			sprintf(res_attr_name, "resource%d", i);
+			res_attr->attr.name = res_attr_name;
 			res_attr->attr.mode = S_IRUSR | S_IWUSR;
 			res_attr->attr.owner = THIS_MODULE;
+			res_attr->size = pci_resource_len(pdev, i);
 			res_attr->mmap = pci_mmap_resource;
 			res_attr->private = &pdev->resource[i];
 			sysfs_create_bin_file(&pdev->dev.kobj, res_attr);
drivers/infiniband/core/sysfs.c +50 -72

188 lines not on this page · raw message