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.
--
Dmitry
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(-)
Index: dtor/lib/kobject_uevent.c
===================================================================
--- dtor.orig/lib/kobject_uevent.c
+++ 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 */
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(-)
Index: dtor/include/linux/sysfs.h
===================================================================
--- dtor.orig/include/linux/sysfs.h
+++ 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)
{
;
}
Index: dtor/fs/sysfs/symlink.c
===================================================================
--- dtor.orig/fs/sysfs/symlink.c
+++ 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;
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(-)
Index: dtor/include/linux/device.h
===================================================================
--- dtor.orig/include/linux/device.h
+++ 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;
Index: dtor/drivers/usb/core/devices.c
===================================================================
--- dtor.orig/drivers/usb/core/devices.c
+++ 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;
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(-)
Index: dtor/include/linux/kobject.h
===================================================================
--- dtor.orig/include/linux/kobject.h
+++ 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 *);
Index: dtor/lib/kobject.c
===================================================================
--- dtor.orig/lib/kobject.c
+++ 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;
Index: dtor/lib/kobject_uevent.c
===================================================================
--- dtor.orig/lib/kobject_uevent.c
+++ 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 */
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(-)
Index: dtor/drivers/base/class.c
===================================================================
--- dtor.orig/drivers/base/class.c
+++ 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);
Index: dtor/include/linux/kobject.h
===================================================================
--- dtor.orig/include/linux/kobject.h
+++ 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);
};
Index: dtor/drivers/base/core.c
===================================================================
--- dtor.orig/drivers/base/core.c
+++ 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);
On Tue, Apr 26, 2005 at 02:29:03AM -0500, Dmitry Torokhov wrote:
> 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.
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
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(-)
Index: dtor/include/linux/sysfs.h
===================================================================
--- dtor.orig/include/linux/sysfs.h
+++ 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;
};
Index: dtor/drivers/pci/pci-sysfs.c
===================================================================
--- dtor.orig/drivers/pci/pci-sysfs.c
+++ 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);
Index: dtor/drivers/infiniband/core/sysfs.c
===================================================================
--- dtor.orig/drivers/infiniband/core/sysfs.c
+++ dtor/drivers/infiniband/core/sysfs.c
@@ -40,9 +40,7 @@ struct ib_port {
struct kobject kobj;
struct ib_device *ibdev;
struct attribute_group gid_group;
- struct attribute **gid_attr;
struct attribute_group pkey_group;
- struct attribute **pkey_attr;
u8 port_num;
};
@@ -60,8 +58,9 @@ struct port_attribute port_attr_##_name
struct port_attribute port_attr_##_name = __ATTR_RO(_name)
struct port_table_attribute {
- struct port_attribute attr;
- int index;
+ struct port_attribute attr;
+ char name[8];
+ int index;
};
static ssize_t port_attr_show(struct kobject *kobj,
@@ -398,17 +397,16 @@ static void ib_port_release(struct kobje
struct attribute *a;
int i;
- for (i = 0; (a = p->gid_attr[i]); ++i) {
- kfree(a->name);
+ for (i = 0; (a = p->gid_group.attrs[i]); ++i)
kfree(a);
- }
- for (i = 0; (a = p->pkey_attr[i]); ++i) {
- kfree(a->name);
+ kfree(p->gid_group.attrs);
+
+ for (i = 0; (a = p->pkey_group.attrs[i]); ++i)
kfree(a);
- }
- kfree(p->gid_attr);
+ kfree(p->pkey_group.attrs);
+
kfree(p);
}
@@ -449,58 +447,45 @@ static int ib_device_hotplug(struct clas
return 0;
}
-static int alloc_group(struct attribute ***attr,
- ssize_t (*show)(struct ib_port *,
- struct port_attribute *, char *buf),
- int len)
+static struct attribute **
+alloc_group_attrs(ssize_t (*show)(struct ib_port *,
+ struct port_attribute *, char *buf),
+ int len)
{
- struct port_table_attribute ***tab_attr =
- (struct port_table_attribute ***) attr;
+ struct attribute **tab_attr;
+ struct port_table_attribute *element;
int i;
- int ret;
-
- *tab_attr = kmalloc((1 + len) * sizeof *tab_attr, GFP_KERNEL);
- if (!*tab_attr)
- return -ENOMEM;
- memset(*tab_attr, 0, (1 + len) * sizeof *tab_attr);
-
- for (i = 0; i < len; ++i) {
- (*tab_attr)[i] = kmalloc(sizeof *(*tab_attr)[i], GFP_KERNEL);
- if (!(*tab_attr)[i]) {
- ret = -ENOMEM;
- goto err;
- }
- memset((*tab_attr)[i], 0, sizeof *(*tab_attr)[i]);
- (*tab_attr)[i]->attr.attr.name = kmalloc(8, GFP_KERNEL);
- if (!(*tab_attr)[i]->attr.attr.name) {
- ret = -ENOMEM;
+ tab_attr = kcalloc(1 + len, sizeof(struct attribute *), GFP_KERNEL);
+ if (!tab_attr)
+ return NULL;
+
+ for (i = 0; i < len; i++) {
+ element = kcalloc(1, sizeof(struct port_table_attribute),
+ GFP_KERNEL);
+ if (!element)
goto err;
- }
- if (snprintf((*tab_attr)[i]->attr.attr.name, 8, "%d", i) >= 8) {
- ret = -ENOMEM;
+ if (snprintf(element->name, sizeof(element->name),
+ "%d", i) >= sizeof(element->name))
goto err;
- }
-
- (*tab_attr)[i]->attr.attr.mode = S_IRUGO;
- (*tab_attr)[i]->attr.attr.owner = THIS_MODULE;
- (*tab_attr)[i]->attr.show = show;
- (*tab_attr)[i]->index = i;
- }
- return 0;
+ element->attr.attr.name = element->name;
+ element->attr.attr.mode = S_IRUGO;
+ element->attr.attr.owner = THIS_MODULE;
+ element->attr.show = show;
+ element->index = i;
-err:
- for (i = 0; i < len; ++i) {
- if ((*tab_attr)[i])
- kfree((*tab_attr)[i]->attr.attr.name);
- kfree((*tab_attr)[i]);
+ tab_attr[i] = &element->attr.attr;
}
- kfree(*tab_attr);
+ return tab_attr;
- return ret;
+err:
+ while (--i >= 0)
+ kfree(tab_attr[i]);
+ kfree(tab_attr);
+ return NULL;
}
static int add_port(struct ib_device *device, int port_num)
@@ -541,23 +526,20 @@ static int add_port(struct ib_device *de
if (ret)
goto err_put;
- ret = alloc_group(&p->gid_attr, show_port_gid, attr.gid_tbl_len);
- if (ret)
- goto err_remove_pma;
-
p->gid_group.name = "gids";
- p->gid_group.attrs = p->gid_attr;
+ p->gid_group.attrs = alloc_group_attrs(show_port_gid, attr.gid_tbl_len);
+ if (!p->gid_group.attrs)
+ goto err_remove_pma;
ret = sysfs_create_group(&p->kobj, &p->gid_group);
if (ret)
goto err_free_gid;
- ret = alloc_group(&p->pkey_attr, show_port_pkey, attr.pkey_tbl_len);
- if (ret)
- goto err_remove_gid;
-
p->pkey_group.name = "pkeys";
- p->pkey_group.attrs = p->pkey_attr;
+ p->pkey_group.attrs = alloc_group_attrs(show_port_pkey,
+ attr.pkey_tbl_len);
+ if (!p->pkey_group.attrs)
+ goto err_remove_gid;
ret = sysfs_create_group(&p->kobj, &p->pkey_group);
if (ret)
@@ -568,23 +550,19 @@ static int add_port(struct ib_device *de
return 0;
err_free_pkey:
- for (i = 0; i < attr.pkey_tbl_len; ++i) {
- kfree(p->pkey_attr[i]->name);
- kfree(p->pkey_attr[i]);
- }
+ for (i = 0; i < attr.pkey_tbl_len; ++i)
+ kfree(p->pkey_group.attrs[i]);
- kfree(p->pkey_attr);
+ kfree(p->pkey_group.attrs);
err_remove_gid:
sysfs_remove_group(&p->kobj, &p->gid_group);
err_free_gid:
- for (i = 0; i < attr.gid_tbl_len; ++i) {
- kfree(p->gid_attr[i]->name);
- kfree(p->gid_attr[i]);
- }
+ for (i = 0; i < attr.gid_tbl_len; ++i)
+ kfree(p->gid_group.attrs[i]);
- kfree(p->gid_attr);
+ kfree(p->gid_group.attrs);
err_remove_pma:
sysfs_remove_group(&p->kobj, &pma_group);