Skip to contentSkip to conversation

discussion [patch] /lib/kobject.c

DISCUSSION walter harms 2 participants 4 messages Mar 10, 2010 – Mar 11, 2010 Kernel janitors

lore
Share & tools
lore
#0 lore raw
Quoted 7 lines · Julia Lawall
Quoted 6 lines

@julia:
i did not check but i can imagine that the pattern A=kmalloc() ... sprintf(a," ",b);

come up at some points inside the kernel since a lot of programmers do not know about asprint().

Quoted 8 lines

I came up with the following. Does it seem ok aside from the stray line over 80 characters?

If it look slike it is going in the right direction, I will make a proer patch. The thing one could be worried about is if the buffer is used for something else, that requires a larger size than that of the string. I haven't yet looked into that.

Hi Julia,
IMHO this is a big improvement.
1. it is easy to understand
1a. the real janitor work now can start, like cleaning statements as shown below
2. strange calculations of length is avoided
2a. simple changes do not cause an overflow anymore
3. this will actually reduce codesize (only by a few bytes but that does not matter)

I do not thing that reuse of the buffer is a problem, i would prefer a policy that uses "%99s" to avoid potential attacks if the source is exposed to userspace somehow. (i thing 99.99% of programmer can live with that).

btw: i did CC the mail to the janitor-ml. I am a janitor only and it is better to have more eyes on the code and make a broader discussion; involving people who can actually make a commit.

NTL your patch was very fast !

re,
wh

/parport/share.c
name = kasprintf(GFP_KERNEL, "parport%d", tmp->portnum = tmp->number);

cachefiles/key.c
key = kasprintf(GFP_KERNEL, "@%02x%c+", (unsigned)csum, 0); /* not to mention the len=0 ;...; len=5 what shows up now. */

Quoted 332 lines

julia

diff -u -p a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c

arch/ia64/pci/pci.c 2010-03-09 23:32:51.000000000 +0100 renamed from arch/ia64/pci/pci.c 2010-03-01 07:52:48.000000000 +0100 +1 -2
@@ -366,11 +366,10 @@ pci_acpi_scan_root(struct acpi_device *d
 		if (!controller->window)
 			goto out2;
 
-		name = kmalloc(16, GFP_KERNEL);
+		name = kasprintf(GFP_KERNEL, "PCI Bus %04x:%02x", domain, bus);
 		if (!name)
 			goto out3;
 
-		sprintf(name, "PCI Bus %04x:%02x", domain, bus);
 		info.bridge = device;
 		info.controller = controller;
 		info.name = name;

diff -u -p a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c

arch/powerpc/platforms/pseries/dlpar.c 2010-03-09 23:32:45.000000000 +0100 renamed from arch/powerpc/platforms/pseries/dlpar.c 2010-01-20 13:04:05.000000000 +0100 +2 -5
@@ -78,13 +78,12 @@ static struct device_node *dlpar_parse_c
 	 * prepend this to the full_name.
 	 */
 	name = (char *)ccwa + ccwa->name_offset;
-	dn->full_name = kmalloc(strlen(name) + 2, GFP_KERNEL);
+	dn->full_name = kasprintf(GFP_KERNEL, "/%s", name);
 	if (!dn->full_name) {
 		kfree(dn);
 		return NULL;
 	}
 
-	sprintf(dn->full_name, "/%s", name);
 	return dn;
 }
 
@@ -409,15 +408,13 @@ static ssize_t dlpar_cpu_probe(const cha
 	 * directory of the device tree.  CPUs actually live in the
 	 * cpus directory so we need to fixup the full_name.
 	 */
-	cpu_name = kzalloc(strlen(dn->full_name) + strlen("/cpus") + 1,
-			   GFP_KERNEL);
+	cpu_name = kasprintf(GFP_KERNEL, "/cpus%s", dn->full_name);
 	if (!cpu_name) {
 		dlpar_free_cc_nodes(dn);
 		rc = -ENOMEM;
 		goto out;
 	}
 
-	sprintf(cpu_name, "/cpus%s", dn->full_name);
 	kfree(dn->full_name);
 	dn->full_name = cpu_name;
 

diff -u -p a/arch/s390/hypfs/inode.c b/arch/s390/hypfs/inode.c

arch/s390/hypfs/inode.c 2010-03-09 23:32:47.000000000 +0100 renamed from arch/s390/hypfs/inode.c 2010-01-29 18:29:07.000000000 +0100 +1 -2
@@ -426,10 +426,9 @@ struct dentry *hypfs_create_str(struct s
 	char *buffer;
 	struct dentry *dentry;
 
-	buffer = kmalloc(strlen(string) + 2, GFP_KERNEL);
+	buffer = kasprintf(GFP_KERNEL, "%s\n", string);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-	sprintf(buffer, "%s\n", string);
 	dentry >  	    hypfs_create_file(sb, dir, name, buffer, S_IFREG | REG_FILE_MODE);
 	if (IS_ERR(dentry)) {

diff -u -p a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c

arch/x86/pci/acpi.c 2010-03-09 23:32:58.000000000 +0100 renamed from arch/x86/pci/acpi.c 2010-03-01 07:52:48.000000000 +0100 +1 -2
@@ -198,10 +198,9 @@ get_current_resources(struct acpi_device
 	if (!info.res)
 		goto res_alloc_fail;
 
-	info.name = kmalloc(16, GFP_KERNEL);
+	info.name = kasprintf(GFP_KERNEL, "PCI Bus %04x:%02x", domain, busnum);
 	if (!info.name)
 		goto name_alloc_fail;
-	sprintf(info.name, "PCI Bus %04x:%02x", domain, busnum);
 
 	info.res_num = 0;
 	acpi_walk_resources(device->handle, METHOD_NAME__CRS, setup_resource,

diff -u -p a/drivers/acpi/video.c b/drivers/acpi/video.c

drivers/acpi/video.c 2010-03-09 23:33:11.000000000 +0100 renamed from drivers/acpi/video.c 2010-03-01 07:52:48.000000000 +0100 +2 -4
@@ -1005,11 +1005,10 @@ static void acpi_video_device_find_cap(s
 		result = acpi_video_init_brightness(device);
 		if (result)
 			return;
-		name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
+		name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
 		if (!name)
 			return;
 
-		sprintf(name, "acpi_video%d", count++);
 		device->backlight = backlight_device_register(name,
 			NULL, device, &acpi_backlight_ops);
 		kfree(name);
@@ -1056,10 +1055,9 @@ static void acpi_video_device_find_cap(s
 		if (device->cap._DCS && device->cap._DSS) {
 			static int count;
 			char *name;
-			name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
+			name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
 			if (!name)
 				return;
-			sprintf(name, "acpi_video%d", count++);
 			device->output_dev = video_output_register(name,
 					NULL, device, &acpi_output_properties);
 			kfree(name);

diff -u -p a/drivers/base/module.c b/drivers/base/module.c

drivers/base/module.c 2010-03-09 23:33:15.000000000 +0100 renamed from drivers/base/module.c 2008-12-07 19:29:06.000000000 +0100 +1 -3
@@ -14,12 +14,10 @@ static char *make_driver_name(struct dev
 {
 	char *driver_name;
 
-	driver_name = kmalloc(strlen(drv->name) + strlen(drv->bus->name) + 2,
-			      GFP_KERNEL);
+	driver_name = kasprintf(GFP_KERNEL, "%s:%s", drv->bus->name, drv->name);
 	if (!driver_name)
 		return NULL;
 
-	sprintf(driver_name, "%s:%s", drv->bus->name, drv->name);
 	return driver_name;
 }
 

diff -u -p a/drivers/char/ppdev.c b/drivers/char/ppdev.c

drivers/char/ppdev.c 2010-03-09 23:33:01.000000000 +0100 renamed from drivers/char/ppdev.c 2009-06-24 21:18:49.000000000 +0200 +1 -3
@@ -286,12 +286,10 @@ static int register_device (int minor, s
 	char *name;
 	int fl;
 
-	name = kmalloc (strlen (CHRDEV) + 3, GFP_KERNEL);
+	name = kasprintf(GFP_KERNEL, CHRDEV"%x", minor);
 	if (name = NULL)
 		return -ENOMEM;
 
-	sprintf (name, CHRDEV "%x", minor);
-
 	port = parport_find_number (minor);
 	if (!port) {
 		printk (KERN_WARNING "%s: no associated port!\n", name);

diff -u -p a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c

drivers/gpu/drm/drm_ioctl.c 2010-03-09 23:33:14.000000000 +0100 renamed from drivers/gpu/drm/drm_ioctl.c 2009-06-24 21:18:49.000000000 +0200 +2 -10
@@ -101,14 +101,10 @@ int drm_setunique(struct drm_device *dev
 
 	master->unique[master->unique_len] = '\0';
 
-	dev->devname = kmalloc(strlen(dev->driver->pci_driver.name) +
-			       strlen(master->unique) + 2, GFP_KERNEL);
+	dev->devname = kasprintf(GFP_KERNEL, "%s@%s", dev->driver->pci_driver.name, master->unique);
 	if (!dev->devname)
 		return -ENOMEM;
 
-	sprintf(dev->devname, "%s@%s", dev->driver->pci_driver.name,
-		master->unique);
-
 	/* Return error if the busid submitted doesn't match the device's actual
 	 * busid.
 	 */
@@ -151,14 +147,10 @@ static int drm_set_busid(struct drm_devi
 	else
 		master->unique_len = len;
 
-	dev->devname = kmalloc(strlen(dev->driver->pci_driver.name) +
-			       master->unique_len + 2, GFP_KERNEL);
+	dev->devname = kasprintf(GFP_KERNEL, "%s@%s", dev->driver->pci_driver.name, master->unique);
 	if (dev->devname = NULL)
 		return -ENOMEM;
 
-	sprintf(dev->devname, "%s@%s", dev->driver->pci_driver.name,
-		master->unique);
-
 	return 0;
 }
 

diff -u -p a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c

drivers/mtd/devices/block2mtd.c 2010-03-09 23:32:50.000000000 +0100 renamed from drivers/mtd/devices/block2mtd.c 2008-12-07 19:29:06.000000000 +0100 +1 -3
@@ -275,12 +275,10 @@ static struct block2mtd_dev *add_device(
 
 	/* Setup the MTD structure */
 	/* make the name contain the block device in */
-	name = kmalloc(sizeof("block2mtd: ") + strlen(devname) + 1,
-			GFP_KERNEL);
+	name = kasprintf(GFP_KERNEL, "block2mtd: %s", devname);
 	if (!name)
 		goto devinit_err;
 
-	sprintf(name, "block2mtd: %s", devname);
 	dev->mtd.name = name;
 
 	dev->mtd.size = dev->blkdev->bd_inode->i_size & PAGE_MASK;

diff -u -p a/drivers/mtd/maps/vmu-flash.c b/drivers/mtd/maps/vmu-flash.c

drivers/mtd/maps/vmu-flash.c 2010-03-09 23:32:48.000000000 +0100 renamed from drivers/mtd/maps/vmu-flash.c 2009-12-03 14:09:47.000000000 +0100 +1 -3
@@ -531,12 +531,10 @@ static void vmu_queryblocks(struct maple
 	part_cur->user_blocks = card->tempA;
 	part_cur->root_block = card->tempB;
 	part_cur->numblocks = card->tempB + 1;
-	part_cur->name = kmalloc(12, GFP_KERNEL);
+	part_cur->name = kasprintf(GFP_KERNEL, "vmu%d.%d.%d", mdev->port, mdev->unit, card->partition);
 	if (!part_cur->name)
 		goto fail_name;
 
-	sprintf(part_cur->name, "vmu%d.%d.%d",
-		mdev->port, mdev->unit, card->partition);
 	mtd_cur = &card->mtd[card->partition];
 	mtd_cur->name = part_cur->name;
 	mtd_cur->type = 8;

diff -u -p a/drivers/parport/share.c b/drivers/parport/share.c

drivers/parport/share.c 2010-03-09 23:33:23.000000000 +0100 renamed from drivers/parport/share.c 2009-06-05 12:16:39.000000000 +0200 +1 -2
@@ -311,7 +311,7 @@ struct parport *parport_register_port(un
 	atomic_set (&tmp->ref_count, 1);
 	INIT_LIST_HEAD(&tmp->full_list);
 
-	name = kmalloc(15, GFP_KERNEL);
+	name = kasprintf(GFP_KERNEL, "parport%d", tmp->portnum = tmp->number);
 	if (!name) {
 		printk(KERN_ERR "parport: memory squeeze\n");
 		kfree(tmp);
@@ -332,7 +332,6 @@ struct parport *parport_register_port(un
 	/*
 	 * Now that the portnum is known finish doing the Init.
 	 */
-	sprintf(name, "parport%d", tmp->portnum = tmp->number);
 	tmp->name = name;
 
 	for (device = 0; device < 5; device++)

diff -u -p a/fs/cachefiles/key.c b/fs/cachefiles/key.c

fs/cachefiles/key.c 2010-03-09 23:35:18.000000000 +0100 renamed from fs/cachefiles/key.c 2009-04-13 16:04:31.000000000 +0200 +1 -2
@@ -78,14 +78,13 @@ char *cachefiles_cook_key(const u8 *raw,
 
 	_debug("max: %d", max);
 
-	key = kmalloc(max, GFP_KERNEL);
+	key = kasprintf(GFP_KERNEL, "@%02x%c+", (unsigned)csum, 0);
 	if (!key)
 		return NULL;
 
 	len = 0;
 
 	/* build the cooked key */
-	sprintf(key, "@%02x%c+", (unsigned) csum, 0);
 	len = 5;
 	mark = len - 1;
 

diff -u -p a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c

fs/hostfs/hostfs_kern.c 2010-03-09 23:35:19.000000000 +0100 renamed from fs/hostfs/hostfs_kern.c 2009-07-04 21:27:34.000000000 +0200 +2 -6
@@ -187,13 +187,12 @@ static char *follow_link(char *link)
 	*(end + 1) = '\0';
 	len = strlen(link) + strlen(name) + 1;
 
-	resolved = kmalloc(len, GFP_KERNEL);
+	resolved = kasprintf(GFP_KERNEL, "%s%s", link, name);
 	if (resolved = NULL) {
 		n = -ENOMEM;
 		goto out_free;
 	}
 
-	sprintf(resolved, "%s%s", link, name);
 	kfree(name);
 	kfree(link);
 	return resolved;
@@ -979,13 +978,10 @@ static int hostfs_fill_sb_common(struct 
 		req_root = "";
 
 	err = -ENOMEM;
-	host_root_path = kmalloc(strlen(root_ino) + 1
-				 + strlen(req_root) + 1, GFP_KERNEL);
+	host_root_path = kasprintf(GFP_KERNEL, "%s/%s", root_ino, req_root);
 	if (host_root_path = NULL)
 		goto out;
 
-	sprintf(host_root_path, "%s/%s", root_ino, req_root);
-
 	root_inode = hostfs_iget(sb);
 	if (IS_ERR(root_inode)) {
 		err = PTR_ERR(root_inode);

diff -u -p a/lib/kobject.c b/lib/kobject.c

lib/kobject.c 2010-03-09 23:35:27.000000000 +0100 renamed from lib/kobject.c 2010-01-29 18:29:09.000000000 +0100 +2 -4
@@ -415,12 +415,11 @@ int kobject_rename(struct kobject *kobj,
 		error = -ENOMEM;
 		goto out;
 	}
-	devpath_string = kmalloc(strlen(devpath) + 15, GFP_KERNEL);
+	devpath_string = kasprintf(GFP_KERNEL, "DEVPATH_OLD=%s", devpath);
 	if (!devpath_string) {
 		error = -ENOMEM;
 		goto out;
 	}
-	sprintf(devpath_string, "DEVPATH_OLD=%s", devpath);
 	envp[0] = devpath_string;
 	envp[1] = NULL;
 
@@ -480,12 +479,11 @@ int kobject_move(struct kobject *kobj, s
 		error = -ENOMEM;
 		goto out;
 	}
-	devpath_string = kmalloc(strlen(devpath) + 15, GFP_KERNEL);
+	devpath_string = kasprintf(GFP_KERNEL, "DEVPATH_OLD=%s", devpath);
 	if (!devpath_string) {
 		error = -ENOMEM;
 		goto out;
 	}
-	sprintf(devpath_string, "DEVPATH_OLD=%s", devpath);
 	envp[0] = devpath_string;
 	envp[1] = NULL;
 	error = sysfs_move_dir(kobj, new_parent);

diff -u -p a/net/atm/proc.c b/net/atm/proc.c

net/atm/proc.c 2010-03-09 23:37:33.000000000 +0100 renamed from net/atm/proc.c 2010-02-16 11:28:04.000000000 +0100 +1 -2
@@ -420,10 +420,9 @@ int atm_proc_dev_register(struct atm_dev
 	if (!digits)
 		digits++;
 
-	dev->proc_name = kmalloc(strlen(dev->type) + digits + 2, GFP_KERNEL);
+	dev->proc_name = kasprintf(GFP_KERNEL, "%s:%d", dev->type, dev->number);
 	if (!dev->proc_name)
 		goto err_out;
-	sprintf(dev->proc_name, "%s:%d", dev->type, dev->number);
 
 	dev->proc_entry = proc_create_data(dev->proc_name, 0, atm_proc_root,
 					   &proc_atm_dev_ops, dev);
#1 lore raw
Quoted 19 lines · walter harms
Quoted 7 lines · Julia Lawall
Quoted 6 lines

@julia:
i did not check but i can imagine that the pattern A=kmalloc() ... sprintf(a," ",b);

come up at some points inside the kernel since a lot of programmers do not know about asprint().

Quoted 8 lines

I came up with the following. Does it seem ok aside from the stray line over 80 characters?

If it look slike it is going in the right direction, I will make a proer patch. The thing one could be worried about is if the buffer is used for something else, that requires a larger size than that of the string. I haven't yet looked into that.

Quoted 7 lines

Hi Julia,
IMHO this is a big improvement.
1. it is easy to understand
1a. the real janitor work now can start, like cleaning statements as shown below
2. strange calculations of length is avoided
2a. simple changes do not cause an overflow anymore
3. this will actually reduce codesize (only by a few bytes but that does not matter)

Thanks for the feedback. I will look into making a proper patch.

julia

Quoted 360 lines

I do not thing that reuse of the buffer is a problem, i would prefer a policy that uses "%99s" to avoid potential attacks if the source is exposed to userspace somehow. (i thing 99.99% of programmer can live with that).

btw: i did CC the mail to the janitor-ml. I am a janitor only and it is better to have more eyes on the code and make a broader discussion; involving people who can actually make a commit.

NTL your patch was very fast !

re,
wh

/parport/share.c
name = kasprintf(GFP_KERNEL, "parport%d", tmp->portnum = tmp->number);

cachefiles/key.c
key = kasprintf(GFP_KERNEL, "@%02x%c+", (unsigned)csum, 0); /* not to mention the len=0 ;...; len=5 what shows up now. */

Quoted 322 lines

julia

diff -u -p a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c

arch/ia64/pci/pci.c 2010-03-09 23:32:51.000000000 +0100 renamed from arch/ia64/pci/pci.c 2010-03-01 07:52:48.000000000 +0100 +1 -2
@@ -366,11 +366,10 @@ pci_acpi_scan_root(struct acpi_device *d
 		if (!controller->window)
 			goto out2;
 
-		name = kmalloc(16, GFP_KERNEL);
+		name = kasprintf(GFP_KERNEL, "PCI Bus %04x:%02x", domain, bus);
 		if (!name)
 			goto out3;
 
-		sprintf(name, "PCI Bus %04x:%02x", domain, bus);
 		info.bridge = device;
 		info.controller = controller;
 		info.name = name;

diff -u -p a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c

arch/powerpc/platforms/pseries/dlpar.c 2010-03-09 23:32:45.000000000 +0100 renamed from arch/powerpc/platforms/pseries/dlpar.c 2010-01-20 13:04:05.000000000 +0100 +2 -5
@@ -78,13 +78,12 @@ static struct device_node *dlpar_parse_c
 	 * prepend this to the full_name.
 	 */
 	name = (char *)ccwa + ccwa->name_offset;
-	dn->full_name = kmalloc(strlen(name) + 2, GFP_KERNEL);
+	dn->full_name = kasprintf(GFP_KERNEL, "/%s", name);
 	if (!dn->full_name) {
 		kfree(dn);
 		return NULL;
 	}
 
-	sprintf(dn->full_name, "/%s", name);
 	return dn;
 }
 
@@ -409,15 +408,13 @@ static ssize_t dlpar_cpu_probe(const cha
 	 * directory of the device tree.  CPUs actually live in the
 	 * cpus directory so we need to fixup the full_name.
 	 */
-	cpu_name = kzalloc(strlen(dn->full_name) + strlen("/cpus") + 1,
-			   GFP_KERNEL);
+	cpu_name = kasprintf(GFP_KERNEL, "/cpus%s", dn->full_name);
 	if (!cpu_name) {
 		dlpar_free_cc_nodes(dn);
 		rc = -ENOMEM;
 		goto out;
 	}
 
-	sprintf(cpu_name, "/cpus%s", dn->full_name);
 	kfree(dn->full_name);
 	dn->full_name = cpu_name;
 

diff -u -p a/arch/s390/hypfs/inode.c b/arch/s390/hypfs/inode.c

arch/s390/hypfs/inode.c 2010-03-09 23:32:47.000000000 +0100 renamed from arch/s390/hypfs/inode.c 2010-01-29 18:29:07.000000000 +0100 +1 -2
@@ -426,10 +426,9 @@ struct dentry *hypfs_create_str(struct s
 	char *buffer;
 	struct dentry *dentry;
 
-	buffer = kmalloc(strlen(string) + 2, GFP_KERNEL);
+	buffer = kasprintf(GFP_KERNEL, "%s\n", string);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
-	sprintf(buffer, "%s\n", string);
 	dentry > >  	    hypfs_create_file(sb, dir, name, buffer, S_IFREG | REG_FILE_MODE);
 	if (IS_ERR(dentry)) {

diff -u -p a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c

arch/x86/pci/acpi.c 2010-03-09 23:32:58.000000000 +0100 renamed from arch/x86/pci/acpi.c 2010-03-01 07:52:48.000000000 +0100 +1 -2
@@ -198,10 +198,9 @@ get_current_resources(struct acpi_device
 	if (!info.res)
 		goto res_alloc_fail;
 
-	info.name = kmalloc(16, GFP_KERNEL);
+	info.name = kasprintf(GFP_KERNEL, "PCI Bus %04x:%02x", domain, busnum);
 	if (!info.name)
 		goto name_alloc_fail;
-	sprintf(info.name, "PCI Bus %04x:%02x", domain, busnum);
 
 	info.res_num = 0;
 	acpi_walk_resources(device->handle, METHOD_NAME__CRS, setup_resource,

diff -u -p a/drivers/acpi/video.c b/drivers/acpi/video.c

drivers/acpi/video.c 2010-03-09 23:33:11.000000000 +0100 renamed from drivers/acpi/video.c 2010-03-01 07:52:48.000000000 +0100 +2 -4
@@ -1005,11 +1005,10 @@ static void acpi_video_device_find_cap(s
 		result = acpi_video_init_brightness(device);
 		if (result)
 			return;
-		name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
+		name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
 		if (!name)
 			return;
 
-		sprintf(name, "acpi_video%d", count++);
 		device->backlight = backlight_device_register(name,
 			NULL, device, &acpi_backlight_ops);
 		kfree(name);
@@ -1056,10 +1055,9 @@ static void acpi_video_device_find_cap(s
 		if (device->cap._DCS && device->cap._DSS) {
 			static int count;
 			char *name;
-			name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
+			name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
 			if (!name)
 				return;
-			sprintf(name, "acpi_video%d", count++);
 			device->output_dev = video_output_register(name,
 					NULL, device, &acpi_output_properties);
 			kfree(name);

diff -u -p a/drivers/base/module.c b/drivers/base/module.c

drivers/base/module.c 2010-03-09 23:33:15.000000000 +0100 renamed from drivers/base/module.c 2008-12-07 19:29:06.000000000 +0100 +1 -3
@@ -14,12 +14,10 @@ static char *make_driver_name(struct dev
 {
 	char *driver_name;
 
-	driver_name = kmalloc(strlen(drv->name) + strlen(drv->bus->name) + 2,
-			      GFP_KERNEL);
+	driver_name = kasprintf(GFP_KERNEL, "%s:%s", drv->bus->name, drv->name);
 	if (!driver_name)
 		return NULL;
 
-	sprintf(driver_name, "%s:%s", drv->bus->name, drv->name);
 	return driver_name;
 }
 

diff -u -p a/drivers/char/ppdev.c b/drivers/char/ppdev.c

drivers/char/ppdev.c 2010-03-09 23:33:01.000000000 +0100 renamed from drivers/char/ppdev.c 2009-06-24 21:18:49.000000000 +0200 +1 -3
@@ -286,12 +286,10 @@ static int register_device (int minor, s
 	char *name;
 	int fl;
 
-	name = kmalloc (strlen (CHRDEV) + 3, GFP_KERNEL);
+	name = kasprintf(GFP_KERNEL, CHRDEV"%x", minor);
 	if (name = NULL)
 		return -ENOMEM;
 
-	sprintf (name, CHRDEV "%x", minor);
-
 	port = parport_find_number (minor);
 	if (!port) {
 		printk (KERN_WARNING "%s: no associated port!\n", name);

diff -u -p a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c

drivers/gpu/drm/drm_ioctl.c 2010-03-09 23:33:14.000000000 +0100 renamed from drivers/gpu/drm/drm_ioctl.c 2009-06-24 21:18:49.000000000 +0200 +2 -10
@@ -101,14 +101,10 @@ int drm_setunique(struct drm_device *dev
 
 	master->unique[master->unique_len] = '\0';
 
-	dev->devname = kmalloc(strlen(dev->driver->pci_driver.name) +
-			       strlen(master->unique) + 2, GFP_KERNEL);
+	dev->devname = kasprintf(GFP_KERNEL, "%s@%s", dev->driver->pci_driver.name, master->unique);
 	if (!dev->devname)
 		return -ENOMEM;
 
-	sprintf(dev->devname, "%s@%s", dev->driver->pci_driver.name,
-		master->unique);
-
 	/* Return error if the busid submitted doesn't match the device's actual
 	 * busid.
 	 */
@@ -151,14 +147,10 @@ static int drm_set_busid(struct drm_devi
 	else
 		master->unique_len = len;
 
-	dev->devname = kmalloc(strlen(dev->driver->pci_driver.name) +
-			       master->unique_len + 2, GFP_KERNEL);
+	dev->devname = kasprintf(GFP_KERNEL, "%s@%s", dev->driver->pci_driver.name, master->unique);
 	if (dev->devname = NULL)
 		return -ENOMEM;
 
-	sprintf(dev->devname, "%s@%s", dev->driver->pci_driver.name,
-		master->unique);
-
 	return 0;
 }
 

diff -u -p a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c

drivers/mtd/devices/block2mtd.c 2010-03-09 23:32:50.000000000 +0100 renamed from drivers/mtd/devices/block2mtd.c 2008-12-07 19:29:06.000000000 +0100 +1 -3
@@ -275,12 +275,10 @@ static struct block2mtd_dev *add_device(
 
 	/* Setup the MTD structure */
 	/* make the name contain the block device in */
-	name = kmalloc(sizeof("block2mtd: ") + strlen(devname) + 1,
-			GFP_KERNEL);
+	name = kasprintf(GFP_KERNEL, "block2mtd: %s", devname);
 	if (!name)
 		goto devinit_err;
 
-	sprintf(name, "block2mtd: %s", devname);
 	dev->mtd.name = name;
 
 	dev->mtd.size = dev->blkdev->bd_inode->i_size & PAGE_MASK;

diff -u -p a/drivers/mtd/maps/vmu-flash.c b/drivers/mtd/maps/vmu-flash.c

drivers/mtd/maps/vmu-flash.c 2010-03-09 23:32:48.000000000 +0100 renamed from drivers/mtd/maps/vmu-flash.c 2009-12-03 14:09:47.000000000 +0100 +1 -3
@@ -531,12 +531,10 @@ static void vmu_queryblocks(struct maple
 	part_cur->user_blocks = card->tempA;
 	part_cur->root_block = card->tempB;
 	part_cur->numblocks = card->tempB + 1;
-	part_cur->name = kmalloc(12, GFP_KERNEL);
+	part_cur->name = kasprintf(GFP_KERNEL, "vmu%d.%d.%d", mdev->port, mdev->unit, card->partition);
 	if (!part_cur->name)
 		goto fail_name;
 
-	sprintf(part_cur->name, "vmu%d.%d.%d",
-		mdev->port, mdev->unit, card->partition);
 	mtd_cur = &card->mtd[card->partition];
 	mtd_cur->name = part_cur->name;
 	mtd_cur->type = 8;

diff -u -p a/drivers/parport/share.c b/drivers/parport/share.c

drivers/parport/share.c 2010-03-09 23:33:23.000000000 +0100 renamed from drivers/parport/share.c 2009-06-05 12:16:39.000000000 +0200 +1 -2
@@ -311,7 +311,7 @@ struct parport *parport_register_port(un
 	atomic_set (&tmp->ref_count, 1);
 	INIT_LIST_HEAD(&tmp->full_list);
 
-	name = kmalloc(15, GFP_KERNEL);
+	name = kasprintf(GFP_KERNEL, "parport%d", tmp->portnum = tmp->number);
 	if (!name) {
 		printk(KERN_ERR "parport: memory squeeze\n");
 		kfree(tmp);
@@ -332,7 +332,6 @@ struct parport *parport_register_port(un
 	/*
 	 * Now that the portnum is known finish doing the Init.
 	 */
-	sprintf(name, "parport%d", tmp->portnum = tmp->number);
 	tmp->name = name;
 
 	for (device = 0; device < 5; device++)

diff -u -p a/fs/cachefiles/key.c b/fs/cachefiles/key.c

fs/cachefiles/key.c 2010-03-09 23:35:18.000000000 +0100 renamed from fs/cachefiles/key.c 2009-04-13 16:04:31.000000000 +0200 +1 -2
@@ -78,14 +78,13 @@ char *cachefiles_cook_key(const u8 *raw,
 
 	_debug("max: %d", max);
 
-	key = kmalloc(max, GFP_KERNEL);
+	key = kasprintf(GFP_KERNEL, "@%02x%c+", (unsigned)csum, 0);
 	if (!key)
 		return NULL;
 
 	len = 0;
 
 	/* build the cooked key */
-	sprintf(key, "@%02x%c+", (unsigned) csum, 0);
 	len = 5;
 	mark = len - 1;
 

diff -u -p a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c

fs/hostfs/hostfs_kern.c 2010-03-09 23:35:19.000000000 +0100 renamed from fs/hostfs/hostfs_kern.c 2009-07-04 21:27:34.000000000 +0200 +2 -6
@@ -187,13 +187,12 @@ static char *follow_link(char *link)
 	*(end + 1) = '\0';
 	len = strlen(link) + strlen(name) + 1;
 
-	resolved = kmalloc(len, GFP_KERNEL);
+	resolved = kasprintf(GFP_KERNEL, "%s%s", link, name);
 	if (resolved = NULL) {
 		n = -ENOMEM;
 		goto out_free;
 	}
 
-	sprintf(resolved, "%s%s", link, name);
 	kfree(name);
 	kfree(link);
 	return resolved;
@@ -979,13 +978,10 @@ static int hostfs_fill_sb_common(struct 
 		req_root = "";
 
 	err = -ENOMEM;
-	host_root_path = kmalloc(strlen(root_ino) + 1
-				 + strlen(req_root) + 1, GFP_KERNEL);
+	host_root_path = kasprintf(GFP_KERNEL, "%s/%s", root_ino, req_root);
 	if (host_root_path = NULL)
 		goto out;
 
-	sprintf(host_root_path, "%s/%s", root_ino, req_root);
-
 	root_inode = hostfs_iget(sb);
 	if (IS_ERR(root_inode)) {
 		err = PTR_ERR(root_inode);

diff -u -p a/lib/kobject.c b/lib/kobject.c

lib/kobject.c 2010-03-09 23:35:27.000000000 +0100 renamed from lib/kobject.c 2010-01-29 18:29:09.000000000 +0100 +2 -4
@@ -415,12 +415,11 @@ int kobject_rename(struct kobject *kobj,
 		error = -ENOMEM;
 		goto out;
 	}
-	devpath_string = kmalloc(strlen(devpath) + 15, GFP_KERNEL);
+	devpath_string = kasprintf(GFP_KERNEL, "DEVPATH_OLD=%s", devpath);
 	if (!devpath_string) {
 		error = -ENOMEM;
 		goto out;
 	}
-	sprintf(devpath_string, "DEVPATH_OLD=%s", devpath);
 	envp[0] = devpath_string;
 	envp[1] = NULL;
 
@@ -480,12 +479,11 @@ int kobject_move(struct kobject *kobj, s
 		error = -ENOMEM;
 		goto out;
 	}
-	devpath_string = kmalloc(strlen(devpath) + 15, GFP_KERNEL);
+	devpath_string = kasprintf(GFP_KERNEL, "DEVPATH_OLD=%s", devpath);
 	if (!devpath_string) {
 		error = -ENOMEM;
 		goto out;
 	}
-	sprintf(devpath_string, "DEVPATH_OLD=%s", devpath);
 	envp[0] = devpath_string;
 	envp[1] = NULL;
 	error = sysfs_move_dir(kobj, new_parent);

diff -u -p a/net/atm/proc.c b/net/atm/proc.c

net/atm/proc.c 2010-03-09 23:37:33.000000000 +0100 renamed from net/atm/proc.c 2010-02-16 11:28:04.000000000 +0100 +0 -0
@@ -420,10 +420,9 @@ int atm_proc_dev_register(struct atm_dev
 	if (!digits)

net/atm/proc.c 2010-03-09 23:37:33.000000000 +0100 int atm_proc_dev_register(struct atm_dev · line 428

Quoted 10 lines
net/atm/proc.c 2010-03-09 23:37:33.000000000 +0100 renamed from net/atm/proc.c 2010-02-16 11:28:04.000000000 +0100 +1 -2
 		digits++;
 
-	dev->proc_name = kmalloc(strlen(dev->type) + digits + 2, GFP_KERNEL);
+	dev->proc_name = kasprintf(GFP_KERNEL, "%s:%d", dev->type, dev->number);
 	if (!dev->proc_name)
 		goto err_out;
-	sprintf(dev->proc_name, "%s:%d", dev->type, dev->number);
 
 	dev->proc_entry = proc_create_data(dev->proc_name, 0, atm_proc_root,
 					   &proc_atm_dev_ops, dev);

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw
In reply to walter harms

I guess something similar could be done with snprintf?

The following code comes from /var/linuxes/linux-next/arch/ia64/pci/pci.c

	len = strlen(info->name) + 32;
        name = kzalloc(len, GFP_KERNEL);
	if (!name) {
                printk(KERN_ERR "PCI: No memory for %s I/O port space name\n",
			info->name);
                goto free_resource;
        }

        min = addr->minimum;
        max = min + addr->address_length - 1;
        if (addr->info.io.translation_type = ACPI_SPARSE_TRANSLATION)
                sparse = 1;

	space_nr = new_space(addr->translation_offset, sparse);
        if (space_nr = ~0)
	        goto free_name;

	base = __pa(io_space[space_nr].mmio_base);
	base_port = IO_SPACE_BASE(space_nr);
        snprintf(name, len, "%s I/O Ports %08lx-%08lx", info->name,
                base_port + min, base_port + max);

julia

#3 lore raw
In reply to walter harms
Quoted 17 lines · Julia Lawall

I guess something similar could be done with snprintf?

The following code comes from /var/linuxes/linux-next/arch/ia64/pci/pci.c

	len = strlen(info->name) + 32;
        name = kzalloc(len, GFP_KERNEL);
	if (!name) {
                printk(KERN_ERR "PCI: No memory for %s I/O port space name\n",
			info->name);
                goto free_resource;
        }

        min = addr->minimum;
        max = min + addr->address_length - 1;
        if (addr->info.io.translation_type = ACPI_SPARSE_TRANSLATION)
                sparse = 1;
Quoted 9 lines
	space_nr = new_space(addr->translation_offset, sparse);
        if (space_nr = ~0)
	        goto free_name;

	base = __pa(io_space[space_nr].mmio_base);
	base_port = IO_SPACE_BASE(space_nr);
        snprintf(name, len, "%s I/O Ports %08lx-%08lx", info->name,
                base_port + min, base_port + max);

yes, i would replace that with:

	 name = kasprintf(GFP_KERNEL,"%s I/O Ports %08lx-%08lx", info->name,
                base_port + min, base_port + max);

          if (!name) {
                printk(KERN_ERR "PCI: No memory for %s I/O port space name\n",
			info->name);
                goto free_resource;
           }

re,
wh