* [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE
@ 2003-05-13 6:26 Ben Collins
2003-05-13 7:10 ` Christoph Hellwig
0 siblings, 1 reply; 19+ messages in thread
From: Ben Collins @ 2003-05-13 6:26 UTC (permalink / raw)
To: Linus Torvalds, linux-kernel
This was causing me all sorts of problems with linux1394's 16-18 byte
long bus_id lengths. The sysfs names were all broken.
This not only makes KOBJ_NAME_LEN match BUS_ID_SIZE, but fixes the
strncpy's in drivers/base/ so that it can't happen again (atleast the
strings will be null terminated).
Index: include/linux/kobject.h
===================================================================
RCS file: /home/scm/linux-2.5/include/linux/kobject.h,v
retrieving revision 1.11
diff -u -r1.11 kobject.h
--- include/linux/kobject.h 12 Apr 2003 01:04:46 -0000 1.11
+++ include/linux/kobject.h 13 May 2003 06:56:30 -0000
@@ -12,7 +12,7 @@
#include <linux/rwsem.h>
#include <asm/atomic.h>
-#define KOBJ_NAME_LEN 16
+#define KOBJ_NAME_LEN 20
struct kobject {
char name[KOBJ_NAME_LEN];
Index: drivers/base/bus.c
===================================================================
RCS file: /home/scm/linux-2.5/drivers/base/bus.c,v
retrieving revision 1.24
diff -u -r1.24 bus.c
--- drivers/base/bus.c 29 Apr 2003 17:30:20 -0000 1.24
+++ drivers/base/bus.c 13 May 2003 06:56:30 -0000
@@ -432,6 +432,7 @@
pr_debug("bus %s: add driver %s\n",bus->name,drv->name);
strncpy(drv->kobj.name,drv->name,KOBJ_NAME_LEN);
+ drv->kobj.name[KOBJ_NAME_LEN-1] = '\0';
drv->kobj.kset = &bus->drivers;
if ((error = kobject_register(&drv->kobj))) {
@@ -541,6 +542,7 @@
int bus_register(struct bus_type * bus)
{
strncpy(bus->subsys.kset.kobj.name,bus->name,KOBJ_NAME_LEN);
+ bus->subsys.kset.kobj.name[KOBJ_NAME_LEN-1] = '\0';
subsys_set_kset(bus,bus_subsys);
subsystem_register(&bus->subsys);
Index: drivers/base/class.c
===================================================================
RCS file: /home/scm/linux-2.5/drivers/base/class.c,v
retrieving revision 1.18
diff -u -r1.18 class.c
--- drivers/base/class.c 11 May 2003 05:00:57 -0000 1.18
+++ drivers/base/class.c 13 May 2003 06:56:30 -0000
@@ -89,6 +89,7 @@
INIT_LIST_HEAD(&cls->interfaces);
strncpy(cls->subsys.kset.kobj.name,cls->name,KOBJ_NAME_LEN);
+ cls->subsys.kset.kobj.name[KOBJ_NAME_LEN-1] = '\0';
subsys_set_kset(cls,class_subsys);
subsystem_register(&cls->subsys);
@@ -259,6 +260,7 @@
/* first, register with generic layer. */
strncpy(class_dev->kobj.name, class_dev->class_id, KOBJ_NAME_LEN);
+ class_dev->kobj.name[KOBJ_NAME_LEN-1] = '\0';
kobj_set_kset_s(class_dev, class_obj_subsys);
if (parent)
class_dev->kobj.parent = &parent->subsys.kset.kobj;
Index: drivers/base/core.c
===================================================================
RCS file: /home/scm/linux-2.5/drivers/base/core.c,v
retrieving revision 1.43
diff -u -r1.43 core.c
--- drivers/base/core.c 29 Apr 2003 17:30:20 -0000 1.43
+++ drivers/base/core.c 13 May 2003 06:56:30 -0000
@@ -214,6 +214,7 @@
/* first, register with generic layer. */
strncpy(dev->kobj.name,dev->bus_id,KOBJ_NAME_LEN);
+ dev->kobj.name[KOBJ_NAME_LEN-1] = '\0';
kobj_set_kset_s(dev,devices_subsys);
if (parent)
dev->kobj.parent = &parent->kobj;
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-13 6:26 [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE Ben Collins @ 2003-05-13 7:10 ` Christoph Hellwig 2003-05-13 7:14 ` Ben Collins 0 siblings, 1 reply; 19+ messages in thread From: Christoph Hellwig @ 2003-05-13 7:10 UTC (permalink / raw) To: Ben Collins; +Cc: Linus Torvalds, linux-kernel On Tue, May 13, 2003 at 02:26:40AM -0400, Ben Collins wrote: > This was causing me all sorts of problems with linux1394's 16-18 byte > long bus_id lengths. The sysfs names were all broken. > > This not only makes KOBJ_NAME_LEN match BUS_ID_SIZE, but fixes the > strncpy's in drivers/base/ so that it can't happen again (atleast the > strings will be null terminated). What about defining BUS_ID_SIZE in terms of KOBJ_NAME_LEN? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-13 7:10 ` Christoph Hellwig @ 2003-05-13 7:14 ` Ben Collins 2003-05-13 15:08 ` Patrick Mochel 0 siblings, 1 reply; 19+ messages in thread From: Ben Collins @ 2003-05-13 7:14 UTC (permalink / raw) To: Christoph Hellwig, Linus Torvalds, linux-kernel On Tue, May 13, 2003 at 08:10:32AM +0100, Christoph Hellwig wrote: > On Tue, May 13, 2003 at 02:26:40AM -0400, Ben Collins wrote: > > This was causing me all sorts of problems with linux1394's 16-18 byte > > long bus_id lengths. The sysfs names were all broken. > > > > This not only makes KOBJ_NAME_LEN match BUS_ID_SIZE, but fixes the > > strncpy's in drivers/base/ so that it can't happen again (atleast the > > strings will be null terminated). > > What about defining BUS_ID_SIZE in terms of KOBJ_NAME_LEN? Ok, then add this in addition to the previous patch. Index: include/linux/device.h =================================================================== RCS file: /home/scm/linux-2.5/include/linux/device.h,v retrieving revision 1.48 diff -u -u -r1.48 device.h --- include/linux/device.h 29 Apr 2003 17:30:20 -0000 1.48 +++ include/linux/device.h 13 May 2003 07:47:39 -0000 @@ -35,7 +35,7 @@ #define DEVICE_NAME_SIZE 50 #define DEVICE_NAME_HALF __stringify(20) /* Less than half to accommodate slop */ #define DEVICE_ID_SIZE 32 -#define BUS_ID_SIZE 20 +#define BUS_ID_SIZE KOBJ_NAME_LEN enum { ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-13 7:14 ` Ben Collins @ 2003-05-13 15:08 ` Patrick Mochel 2003-05-16 0:20 ` Resend " Ben Collins 0 siblings, 1 reply; 19+ messages in thread From: Patrick Mochel @ 2003-05-13 15:08 UTC (permalink / raw) To: Ben Collins; +Cc: Christoph Hellwig, Linus Torvalds, linux-kernel On Tue, 13 May 2003, Ben Collins wrote: > On Tue, May 13, 2003 at 08:10:32AM +0100, Christoph Hellwig wrote: > > On Tue, May 13, 2003 at 02:26:40AM -0400, Ben Collins wrote: > > > This was causing me all sorts of problems with linux1394's 16-18 byte > > > long bus_id lengths. The sysfs names were all broken. > > > > > > This not only makes KOBJ_NAME_LEN match BUS_ID_SIZE, but fixes the > > > strncpy's in drivers/base/ so that it can't happen again (atleast the > > > strings will be null terminated). > > > > What about defining BUS_ID_SIZE in terms of KOBJ_NAME_LEN? > > Ok, then add this in addition to the previous patch. I'll add this, and sync with Linus this week, if he doesn't pick it up. Thanks, -pat ^ permalink raw reply [flat|nested] 19+ messages in thread
* Resend [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-13 15:08 ` Patrick Mochel @ 2003-05-16 0:20 ` Ben Collins 2003-05-16 18:43 ` Felipe Alfaro Solana 2003-05-25 0:07 ` Ben Collins 0 siblings, 2 replies; 19+ messages in thread From: Ben Collins @ 2003-05-16 0:20 UTC (permalink / raw) To: Patrick Mochel; +Cc: Christoph Hellwig, Linus Torvalds, linux-kernel On Tue, May 13, 2003 at 08:08:35AM -0700, Patrick Mochel wrote: > > On Tue, 13 May 2003, Ben Collins wrote: > > > On Tue, May 13, 2003 at 08:10:32AM +0100, Christoph Hellwig wrote: > > > On Tue, May 13, 2003 at 02:26:40AM -0400, Ben Collins wrote: > > > > This was causing me all sorts of problems with linux1394's 16-18 byte > > > > long bus_id lengths. The sysfs names were all broken. > > > > > > > > This not only makes KOBJ_NAME_LEN match BUS_ID_SIZE, but fixes the > > > > strncpy's in drivers/base/ so that it can't happen again (atleast the > > > > strings will be null terminated). > > > > > > What about defining BUS_ID_SIZE in terms of KOBJ_NAME_LEN? > > > > Ok, then add this in addition to the previous patch. > > I'll add this, and sync with Linus this week, if he doesn't pick it up. *sigh* Patrick, you accepted the BUS_ID_SIZE change without the original patch, so now BUS_ID_SIZE is back to 16 bytes. Linus, please apply this patch to get things right again. ------- This was causing me all sorts of problems with linux1394's 16-18 byte long bus_id lengths. The sysfs names were all broken. This not only makes KOBJ_NAME_LEN match BUS_ID_SIZE, but fixes the strncpy's in drivers/base/ so that it can't happen again (atleast the strings will be null terminated). Index: include/linux/kobject.h =================================================================== RCS file: /home/scm/linux-2.5/include/linux/kobject.h,v retrieving revision 1.11 diff -u -r1.11 kobject.h --- include/linux/kobject.h 12 Apr 2003 01:04:46 -0000 1.11 +++ include/linux/kobject.h 13 May 2003 06:56:30 -0000 @@ -12,7 +12,7 @@ #include <linux/rwsem.h> #include <asm/atomic.h> -#define KOBJ_NAME_LEN 16 +#define KOBJ_NAME_LEN 20 struct kobject { char name[KOBJ_NAME_LEN]; Index: drivers/base/bus.c =================================================================== RCS file: /home/scm/linux-2.5/drivers/base/bus.c,v retrieving revision 1.24 diff -u -r1.24 bus.c --- drivers/base/bus.c 29 Apr 2003 17:30:20 -0000 1.24 +++ drivers/base/bus.c 13 May 2003 06:56:30 -0000 @@ -432,6 +432,7 @@ pr_debug("bus %s: add driver %s\n",bus->name,drv->name); strncpy(drv->kobj.name,drv->name,KOBJ_NAME_LEN); + drv->kobj.name[KOBJ_NAME_LEN-1] = '\0'; drv->kobj.kset = &bus->drivers; if ((error = kobject_register(&drv->kobj))) { @@ -541,6 +542,7 @@ int bus_register(struct bus_type * bus) { strncpy(bus->subsys.kset.kobj.name,bus->name,KOBJ_NAME_LEN); + bus->subsys.kset.kobj.name[KOBJ_NAME_LEN-1] = '\0'; subsys_set_kset(bus,bus_subsys); subsystem_register(&bus->subsys); Index: drivers/base/class.c =================================================================== RCS file: /home/scm/linux-2.5/drivers/base/class.c,v retrieving revision 1.18 diff -u -r1.18 class.c --- drivers/base/class.c 11 May 2003 05:00:57 -0000 1.18 +++ drivers/base/class.c 13 May 2003 06:56:30 -0000 @@ -89,6 +89,7 @@ INIT_LIST_HEAD(&cls->interfaces); strncpy(cls->subsys.kset.kobj.name,cls->name,KOBJ_NAME_LEN); + cls->subsys.kset.kobj.name[KOBJ_NAME_LEN-1] = '\0'; subsys_set_kset(cls,class_subsys); subsystem_register(&cls->subsys); @@ -259,6 +260,7 @@ /* first, register with generic layer. */ strncpy(class_dev->kobj.name, class_dev->class_id, KOBJ_NAME_LEN); + class_dev->kobj.name[KOBJ_NAME_LEN-1] = '\0'; kobj_set_kset_s(class_dev, class_obj_subsys); if (parent) class_dev->kobj.parent = &parent->subsys.kset.kobj; Index: drivers/base/core.c =================================================================== RCS file: /home/scm/linux-2.5/drivers/base/core.c,v retrieving revision 1.43 diff -u -r1.43 core.c --- drivers/base/core.c 29 Apr 2003 17:30:20 -0000 1.43 +++ drivers/base/core.c 13 May 2003 06:56:30 -0000 @@ -214,6 +214,7 @@ /* first, register with generic layer. */ strncpy(dev->kobj.name,dev->bus_id,KOBJ_NAME_LEN); + dev->kobj.name[KOBJ_NAME_LEN-1] = '\0'; kobj_set_kset_s(dev,devices_subsys); if (parent) dev->kobj.parent = &parent->kobj; ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Resend [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-16 0:20 ` Resend " Ben Collins @ 2003-05-16 18:43 ` Felipe Alfaro Solana 2003-05-25 0:07 ` Ben Collins 1 sibling, 0 replies; 19+ messages in thread From: Felipe Alfaro Solana @ 2003-05-16 18:43 UTC (permalink / raw) To: Ben Collins; +Cc: Patrick Mochel, Christoph Hellwig, Linus Torvalds, LKML On Fri, 2003-05-16 at 02:20, Ben Collins wrote: > On Tue, May 13, 2003 at 08:08:35AM -0700, Patrick Mochel wrote: > > > > On Tue, 13 May 2003, Ben Collins wrote: > > > > > On Tue, May 13, 2003 at 08:10:32AM +0100, Christoph Hellwig wrote: > > > > On Tue, May 13, 2003 at 02:26:40AM -0400, Ben Collins wrote: > > > > > This was causing me all sorts of problems with linux1394's 16-18 byte > > > > > long bus_id lengths. The sysfs names were all broken. > > > > > > > > > > This not only makes KOBJ_NAME_LEN match BUS_ID_SIZE, but fixes the > > > > > strncpy's in drivers/base/ so that it can't happen again (atleast the > > > > > strings will be null terminated). > > > > > > > > What about defining BUS_ID_SIZE in terms of KOBJ_NAME_LEN? > > > > > > Ok, then add this in addition to the previous patch. > > > > I'll add this, and sync with Linus this week, if he doesn't pick it up. > > *sigh* Patrick, you accepted the BUS_ID_SIZE change without the original > patch, so now BUS_ID_SIZE is back to 16 bytes. > > Linus, please apply this patch to get things right again. Well, it seems this patch has bad interaction with the Yamaha ALSA sound driver (aka snd-ymfpci). Don't know why, but changing KOBJ_NAME_LEN from 16 to 20, causes snd-ymfpci.ko to oops. If you have the time, please take a look at another thread in the LKML called "pccard oops while booting". ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Resend [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-16 0:20 ` Resend " Ben Collins 2003-05-16 18:43 ` Felipe Alfaro Solana @ 2003-05-25 0:07 ` Ben Collins 2003-05-25 3:52 ` Linus Torvalds 2003-05-25 8:02 ` Russell King 1 sibling, 2 replies; 19+ messages in thread From: Ben Collins @ 2003-05-25 0:07 UTC (permalink / raw) To: Patrick Mochel; +Cc: Linus Torvalds, linux-kernel Given that the problem with KOBJ_NAME_LEN == 20 affecting one snd driver has so far only been explained as a compiler bug, can I suggest this patch be applied? Even aside from the KOBJ_NAME_LEN == 20, the snprintf changes will keep things from breaking in other ways that are current now. Index: include/linux/kobject.h =================================================================== --- include/linux/kobject.h (revision 10014) +++ include/linux/kobject.h (working copy) @@ -12,7 +12,7 @@ #include <linux/rwsem.h> #include <asm/atomic.h> -#define KOBJ_NAME_LEN 16 +#define KOBJ_NAME_LEN 20 struct kobject { char name[KOBJ_NAME_LEN]; Index: drivers/base/class.c =================================================================== --- drivers/base/class.c (revision 10014) +++ drivers/base/class.c (working copy) @@ -87,8 +87,9 @@ INIT_LIST_HEAD(&cls->children); INIT_LIST_HEAD(&cls->interfaces); - - strncpy(cls->subsys.kset.kobj.name,cls->name,KOBJ_NAME_LEN); + + snprintf(cls->subsys.kset.kobj.name, KOBJ_NAME_LEN, "%s", + cls->name); subsys_set_kset(cls,class_subsys); subsystem_register(&cls->subsys); @@ -258,7 +259,7 @@ class_dev->class_id); /* first, register with generic layer. */ - strncpy(class_dev->kobj.name, class_dev->class_id, KOBJ_NAME_LEN); + snprintf(class_dev->kobj.name, KOBJ_NAME_LEN, "%s", class_dev->class_id); kobj_set_kset_s(class_dev, class_obj_subsys); if (parent) class_dev->kobj.parent = &parent->subsys.kset.kobj; Index: drivers/base/core.c =================================================================== --- drivers/base/core.c (revision 10014) +++ drivers/base/core.c (working copy) @@ -211,7 +211,7 @@ dev->bus_id, dev->name); /* first, register with generic layer. */ - strncpy(dev->kobj.name,dev->bus_id,KOBJ_NAME_LEN); + snprintf(dev->kobj.name, KOBJ_NAME_LEN, "%s", dev->bus_id); if (parent) dev->kobj.parent = &parent->kobj; Index: drivers/base/bus.c =================================================================== --- drivers/base/bus.c (revision 10014) +++ drivers/base/bus.c (working copy) @@ -431,7 +431,7 @@ if (bus) { pr_debug("bus %s: add driver %s\n",bus->name,drv->name); - strncpy(drv->kobj.name,drv->name,KOBJ_NAME_LEN); + snprintf(drv->kobj.name, KOBJ_NAME_LEN, "%s", drv->name); drv->kobj.kset = &bus->drivers; if ((error = kobject_register(&drv->kobj))) { @@ -540,7 +540,8 @@ */ int bus_register(struct bus_type * bus) { - strncpy(bus->subsys.kset.kobj.name,bus->name,KOBJ_NAME_LEN); + snprintf(bus->subsys.kset.kobj.name, KOBJ_NAME_LEN, "%s", + bus->name); subsys_set_kset(bus,bus_subsys); subsystem_register(&bus->subsys); ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Resend [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-25 0:07 ` Ben Collins @ 2003-05-25 3:52 ` Linus Torvalds 2003-05-25 3:10 ` Ben Collins ` (4 more replies) 2003-05-25 8:02 ` Russell King 1 sibling, 5 replies; 19+ messages in thread From: Linus Torvalds @ 2003-05-25 3:52 UTC (permalink / raw) To: Ben Collins; +Cc: Patrick Mochel, linux-kernel On Sat, 24 May 2003, Ben Collins wrote: > > Given that the problem with KOBJ_NAME_LEN == 20 affecting one snd driver > has so far only been explained as a compiler bug, can I suggest this > patch be applied? Even aside from the KOBJ_NAME_LEN == 20, the snprintf > changes will keep things from breaking in other ways that are current > now. I hate using snprintf() for this kind of mindless string copy opertation. Yeah, "strncpy()" is a frigging disaster when it comes to '\0', in many ways. We should probably disallow using strncpy(), and aim for a _sane_ implementation that does what we actually want (none of that zero-padding crap, and _always_ put a NUL at the end). I bet that is what most current strncpy() users actually would want. But switching it over to "snprintf()" is overkill. How about just adding a sane int copy_string(char *dest, const char *src, int len) { int size; if (!len) return 0; size = strlen(src); if (size >= len) size = len-1; memcpy(dest, src, size); dest[size] = '\0'; return size; } which is what pretty much everybody really _wants_ to have anyway? We should deprecate "strncpy()" within the kernel entirely. Linus ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Resend [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-25 3:52 ` Linus Torvalds @ 2003-05-25 3:10 ` Ben Collins 2003-05-25 12:03 ` Adam Sampson ` (3 subsequent siblings) 4 siblings, 0 replies; 19+ messages in thread From: Ben Collins @ 2003-05-25 3:10 UTC (permalink / raw) To: Linus Torvalds; +Cc: Patrick Mochel, linux-kernel > which is what pretty much everybody really _wants_ to have anyway? We > should deprecate "strncpy()" within the kernel entirely. Amen. If you could atleast apply the KOBJ_NAME_LEN part of the patch, I'll start doing a much wider scope patch of replacing strncpy in the whole kernel. -- Debian - http://www.debian.org/ Linux 1394 - http://www.linux1394.org/ Subversion - http://subversion.tigris.org/ Deqo - http://www.deqo.com/ ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Resend [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-25 3:52 ` Linus Torvalds 2003-05-25 3:10 ` Ben Collins @ 2003-05-25 12:03 ` Adam Sampson 2003-05-25 17:10 ` Linus Torvalds 2003-05-25 15:51 ` Matt Mackall ` (2 subsequent siblings) 4 siblings, 1 reply; 19+ messages in thread From: Adam Sampson @ 2003-05-25 12:03 UTC (permalink / raw) To: Linus Torvalds; +Cc: Ben Collins, Patrick Mochel, linux-kernel Linus Torvalds <torvalds@transmeta.com> writes: > How about just adding a sane > int copy_string(char *dest, const char *src, int len) [...] If you're going to do this, it might make sense to call it "strlcpy" for consistency with the OpenBSD-introduced function of the same name that's getting included in a lot of userspace these days... <http://www.courtesan.com/todd/papers/strlcpy.html> -- Adam Sampson <azz@us-lot.org> <http://azz.us-lot.org/> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Resend [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-25 12:03 ` Adam Sampson @ 2003-05-25 17:10 ` Linus Torvalds 2003-05-25 16:40 ` Ben Collins 0 siblings, 1 reply; 19+ messages in thread From: Linus Torvalds @ 2003-05-25 17:10 UTC (permalink / raw) To: Adam Sampson; +Cc: Ben Collins, Patrick Mochel, linux-kernel On 25 May 2003, Adam Sampson wrote: > > If you're going to do this, it might make sense to call it "strlcpy" > for consistency with the OpenBSD-introduced function of the same name > that's getting included in a lot of userspace these days... Sure, done. I'll check it in asap (and I'll make the devfs parts that Ben was unhappy about use it too). Somebody (else ;) should probably go through our current uses of strncpy() and see if they make sense. Some of them probably do, but I suspect anything name/path related does not. Linus ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Resend [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-25 17:10 ` Linus Torvalds @ 2003-05-25 16:40 ` Ben Collins 0 siblings, 0 replies; 19+ messages in thread From: Ben Collins @ 2003-05-25 16:40 UTC (permalink / raw) To: Linus Torvalds; +Cc: Adam Sampson, Patrick Mochel, linux-kernel On Sun, May 25, 2003 at 10:10:56AM -0700, Linus Torvalds wrote: > > On 25 May 2003, Adam Sampson wrote: > > > > If you're going to do this, it might make sense to call it "strlcpy" > > for consistency with the OpenBSD-introduced function of the same name > > that's getting included in a lot of userspace these days... > > Sure, done. I'll check it in asap (and I'll make the devfs parts that Ben > was unhappy about use it too). > > Somebody (else ;) should probably go through our current uses of strncpy() > and see if they make sense. Some of them probably do, but I suspect > anything name/path related does not. Please hold off. I have a better patch that includes strlcat. Actually tested. -- Debian - http://www.debian.org/ Linux 1394 - http://www.linux1394.org/ Subversion - http://subversion.tigris.org/ Deqo - http://www.deqo.com/ ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Resend [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-25 3:52 ` Linus Torvalds 2003-05-25 3:10 ` Ben Collins 2003-05-25 12:03 ` Adam Sampson @ 2003-05-25 15:51 ` Matt Mackall 2003-05-25 17:25 ` Riley Williams 2003-05-25 18:13 ` Valdis.Kletnieks 2003-05-25 16:41 ` Ben Collins 2003-07-11 9:50 ` Rogier Wolff 4 siblings, 2 replies; 19+ messages in thread From: Matt Mackall @ 2003-05-25 15:51 UTC (permalink / raw) To: Linus Torvalds; +Cc: Ben Collins, Patrick Mochel, linux-kernel On Sat, May 24, 2003 at 08:52:53PM -0700, Linus Torvalds wrote: > > How about just adding a sane > > int copy_string(char *dest, const char *src, int len) > { > int size; > > if (!len) > return 0; > size = strlen(src); > if (size >= len) > size = len-1; > memcpy(dest, src, size); > dest[size] = '\0'; > return size; > } The return value here isn't particularly useful. The OpenBSD strlcpy/strlcat variant tell you how big the result should have been so that you can realloc if need be. -- Matt Mackall : http://www.selenic.com : of or relating to the moon ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: Resend [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-25 15:51 ` Matt Mackall @ 2003-05-25 17:25 ` Riley Williams 2003-05-25 18:13 ` Valdis.Kletnieks 1 sibling, 0 replies; 19+ messages in thread From: Riley Williams @ 2003-05-25 17:25 UTC (permalink / raw) To: Matt Mackall, Linus Torvalds; +Cc: Ben Collins, Patrick Mochel, linux-kernel Hi Matt. >> How about just adding a sane >> >> int copy_string(char *dest, const char *src, int len) >> { >> int size; >> >> if (!len) >> return 0; >> size = strlen(src); >> if (size >= len) >> size = len-1; >> memcpy(dest, src, size); >> dest[size] = '\0'; >> return size; >> } > The return value here isn't particularly useful. The OpenBSD > strlcpy/strlcat variant tell you how big the result should have been > so that you can realloc if need be. Something along the lines of... int strlcpy(char *tgt, char *src, int len) { int size = strlen(src); if (size < len) strcpy(tgt, src); else { memcpy(tgt, src, len-1); tgt[len] = '\0'; } return size; } ...reindented according to standards (which I don't have to hand). Best wishes from Riley. --- * Nothing as pretty as a smile, nothing as ugly as a frown. --- Outgoing mail is certified Virus Free. Checked by AVG anti-virus system (http://www.grisoft.com). Version: 6.0.483 / Virus Database: 279 - Release Date: 19-May-2003 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Resend [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-25 15:51 ` Matt Mackall 2003-05-25 17:25 ` Riley Williams @ 2003-05-25 18:13 ` Valdis.Kletnieks 2003-05-25 23:42 ` Matt Mackall 1 sibling, 1 reply; 19+ messages in thread From: Valdis.Kletnieks @ 2003-05-25 18:13 UTC (permalink / raw) To: Matt Mackall; +Cc: linux-kernel [-- Attachment #1: Type: text/plain, Size: 727 bytes --] On Sun, 25 May 2003 10:51:02 CDT, Matt Mackall said: > The return value here isn't particularly useful. The OpenBSD > strlcpy/strlcat variant tell you how big the result should have been > so that you can realloc if need be. A quick grep of the current source tree seems to indicate that there aren't any uses of 'strncpy' that actually save or check the return value. As such, actually *using* the return value would make for a job for the kernel janitors, to actually do something useful at all 650 or so uses. Given that the kernel probably *shouldn't* be trying to strlcpy() into a destination that it won't fit, it may be more useful to do a BUG_ON() or similar (feel free to use a 'goto too_long;' if you prefer ;) [-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Resend [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-25 18:13 ` Valdis.Kletnieks @ 2003-05-25 23:42 ` Matt Mackall 0 siblings, 0 replies; 19+ messages in thread From: Matt Mackall @ 2003-05-25 23:42 UTC (permalink / raw) To: Valdis.Kletnieks; +Cc: linux-kernel On Sun, May 25, 2003 at 02:13:02PM -0400, Valdis.Kletnieks@vt.edu wrote: > On Sun, 25 May 2003 10:51:02 CDT, Matt Mackall said: > > > The return value here isn't particularly useful. The OpenBSD > > strlcpy/strlcat variant tell you how big the result should have been > > so that you can realloc if need be. > > A quick grep of the current source tree seems to indicate that there aren't > any uses of 'strncpy' that actually save or check the return value. > > As such, actually *using* the return value would make for a job for the > kernel janitors, to actually do something useful at all 650 or so uses. The kernel, fortunately, isn't Perl or the like and really isn't interested in strings outside the context of things like pathnames, which realistically have to have finite limits. So while there probably aren't a lot of uses for a return val, for the places where there are, min(bufsize, optimalsize) is going to be less useful than just optimalsize as you already know bufsize. > Given that the kernel probably *shouldn't* be trying to strlcpy() into > a destination that it won't fit, it may be more useful to do a BUG_ON() > or similar (feel free to use a 'goto too_long;' if you prefer ;) Outside of pathnames type things, where there's a well-defined return code (ENAMETOOLONG), we probably end up not caring overmuch about truncation.. -- Matt Mackall : http://www.selenic.com : of or relating to the moon ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Resend [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-25 3:52 ` Linus Torvalds ` (2 preceding siblings ...) 2003-05-25 15:51 ` Matt Mackall @ 2003-05-25 16:41 ` Ben Collins 2003-07-11 9:50 ` Rogier Wolff 4 siblings, 0 replies; 19+ messages in thread From: Ben Collins @ 2003-05-25 16:41 UTC (permalink / raw) To: Linus Torvalds; +Cc: Patrick Mochel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 701 bytes --] > > which is what pretty much everybody really _wants_ to have anyway? We > should deprecate "strncpy()" within the kernel entirely. Let's try this one for size. First patch adds strlcpy and strlcat. The second patch applies to my initial offender, drivers/base. I'll do obvious conversions of the rest of tree over the next week and hope that the architectures will be able to do optimized versions in the same time frame. If everything works out ok, strncpy (and maybe strncat) can be either marked deprecated or just removed altogether. -- Debian - http://www.debian.org/ Linux 1394 - http://www.linux1394.org/ Subversion - http://subversion.tigris.org/ Deqo - http://www.deqo.com/ [-- Attachment #2: strlcpy-strlcat.diff --] [-- Type: text/plain, Size: 5590 bytes --] Index: kernel/ksyms.c =================================================================== --- kernel/ksyms.c (revision 10041) +++ kernel/ksyms.c (working copy) @@ -578,6 +578,8 @@ EXPORT_SYMBOL(strnicmp); EXPORT_SYMBOL(strspn); EXPORT_SYMBOL(strsep); +EXPORT_SYMBOL(strlcat); +EXPORT_SYMBOL(strlcpy); /* software interrupts */ EXPORT_SYMBOL(tasklet_init); Index: include/linux/string.h =================================================================== --- include/linux/string.h (revision 10041) +++ include/linux/string.h (working copy) @@ -25,12 +25,18 @@ #ifndef __HAVE_ARCH_STRCPY extern char * strcpy(char *,const char *); #endif +#ifndef __HAVE_ARCH_STRLCPY +extern size_t strlcpy(char *dest, const char *src, size_t size); +#endif #ifndef __HAVE_ARCH_STRNCPY extern char * strncpy(char *,const char *, __kernel_size_t); #endif #ifndef __HAVE_ARCH_STRCAT extern char * strcat(char *, const char *); #endif +#ifndef __HAVE_ARCH_STRLCAT +extern size_t strlcat(char *dest, const char *src, size_t size); +#endif #ifndef __HAVE_ARCH_STRNCAT extern char * strncat(char *, const char *, __kernel_size_t); #endif Index: lib/string.c =================================================================== --- lib/string.c (revision 10041) +++ lib/string.c (working copy) @@ -17,8 +17,40 @@ * * Sat Feb 09 2002, Jason Thomas <jason@topic.com.au>, * Matthew Hawkins <matt@mh.dropbear.id.au> * - Kissed strtok() goodbye + * + * * Sun May 25 2003, Ben Collins <bcollins@debian.org> + * - Added strlcpy and strlcat, which will replace strncpy and strncat + * since they are safer and guarantee a NUL-terminated dest. */ - + +/* Following applies to strlcpy and strlcat */ +/* + * Copyright (c) 2001 Richard Kettlewell <rjk@greenend.org.uk> + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. The name of the author may not be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY + * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL + * THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, + * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; + * OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, + * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR + * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + #include <linux/types.h> #include <linux/string.h> #include <linux/ctype.h> @@ -72,6 +104,35 @@ } #endif +#ifndef __HAVE_ARCH_STRLCPY +/** + * strlcpy - Copy a length-limited, %NUL-terminated string + * @dest: Where to copy the string to + * @src: Where to copy the string from + * @size: The size of the @dest buffer + * + * Note that unlike strncpy, the result is always guaranteed to be + * %NUL-terminated but is not %NUL-padded. This is much safer than + * strncpy. The return value is basically strlen(src), which means you can + * use it to check if the result should have been longer than @size, + * meaning it was truncated. + */ +size_t strlcpy(char *dest, const char *src, size_t size) +{ + size_t l = strlen(src); + + if (l >= size) { + if (size) { + memcpy(dest, src, size - 1); + dest[size - 1] = 0; + } + } else if (l) + memcpy(dest, src, l + 1); + + return l; +} +#endif + #ifndef __HAVE_ARCH_STRNCPY /** * strncpy - Copy a length-limited, %NUL-terminated string @@ -113,6 +174,53 @@ } #endif +#ifndef __HAVE_ARCH_STRLCAT +/** + * strlcat - Append a length-limited, %NUL-terminated string to another + * @dest: The string to be appended to + * @src: The string to append to it + * @size: The size of the @dest buffer + * + * Note that even though strncat %NUL-terminates, this function is more + * useful since it is easier to check for truncation. + * + * Unlink strncat, @size is the total size of @dest, not just the amount + * left. The return value is strlen(src) + strlen(initial dest)). + * + * If the return value is greater than @size, truncation occured. + */ +size_t strlcat(char *dest, const char *src, size_t size) +{ + size_t sl = strlen(src); + size_t dl, tl; + + /* strlcpy wont give us NULL termination if size is 0, so we can't + * check strlen(dest) if someone does this scenario: + * + * strlcpy(dest, "My ", 0); + * strlcat(dest, "string", 0); + * + * Stupid, I know, but just a check. + */ + if (!size) + return sl; + + dl = strlen(dest); + tl = sl + dl; + + if (tl >= size) { + /* Truncation occurs, but check for room */ + if (size > dl) { + memcpy(dest + dl, src, size - dl - 1); + dest[size - 1] = 0; + } + } else + memcpy(dest + dl, src, sl + 1); + + return tl; +} +#endif + #ifndef __HAVE_ARCH_STRNCAT /** * strncat - Append a length-limited, %NUL-terminated string to another [-- Attachment #3: base-strlcpy.diff --] [-- Type: text/plain, Size: 2963 bytes --] Index: drivers/base/class.c =================================================================== --- drivers/base/class.c (revision 10041) +++ drivers/base/class.c (working copy) @@ -87,8 +87,8 @@ INIT_LIST_HEAD(&cls->children); INIT_LIST_HEAD(&cls->interfaces); - - strncpy(cls->subsys.kset.kobj.name,cls->name,KOBJ_NAME_LEN); + + strlcpy(cls->subsys.kset.kobj.name, cls->name, KOBJ_NAME_LEN); subsys_set_kset(cls,class_subsys); subsystem_register(&cls->subsys); @@ -258,7 +258,7 @@ class_dev->class_id); /* first, register with generic layer. */ - strncpy(class_dev->kobj.name, class_dev->class_id, KOBJ_NAME_LEN); + strlcpy(class_dev->kobj.name, class_dev->class_id, KOBJ_NAME_LEN); kobj_set_kset_s(class_dev, class_obj_subsys); if (parent) class_dev->kobj.parent = &parent->subsys.kset.kobj; Index: drivers/base/sys.c =================================================================== --- drivers/base/sys.c (revision 10041) +++ drivers/base/sys.c (working copy) @@ -63,8 +63,8 @@ error = device_register(&root->dev); if (!error) { - strncpy(root->sysdev.bus_id,"sys",BUS_ID_SIZE); - strncpy(root->sysdev.name,"System Bus",DEVICE_NAME_SIZE); + strlcpy(root->sysdev.bus_id,"sys",BUS_ID_SIZE); + strlcpy(root->sysdev.name,"System Bus",DEVICE_NAME_SIZE); root->sysdev.parent = &root->dev; error = device_register(&root->sysdev); }; Index: drivers/base/core.c =================================================================== --- drivers/base/core.c (revision 10041) +++ drivers/base/core.c (working copy) @@ -211,7 +211,7 @@ dev->bus_id, dev->name); /* first, register with generic layer. */ - strncpy(dev->kobj.name,dev->bus_id,KOBJ_NAME_LEN); + strlcpy(dev->kobj.name, dev->bus_id, KOBJ_NAME_LEN); if (parent) dev->kobj.parent = &parent->kobj; Index: drivers/base/bus.c =================================================================== --- drivers/base/bus.c (revision 10041) +++ drivers/base/bus.c (working copy) @@ -431,7 +431,7 @@ if (bus) { pr_debug("bus %s: add driver %s\n",bus->name,drv->name); - strncpy(drv->kobj.name,drv->name,KOBJ_NAME_LEN); + strlcpy(drv->kobj.name, drv->name, KOBJ_NAME_LEN); drv->kobj.kset = &bus->drivers; if ((error = kobject_register(&drv->kobj))) { @@ -540,15 +540,15 @@ */ int bus_register(struct bus_type * bus) { - strncpy(bus->subsys.kset.kobj.name,bus->name,KOBJ_NAME_LEN); + strlcpy(bus->subsys.kset.kobj.name, bus->name, KOBJ_NAME_LEN); subsys_set_kset(bus,bus_subsys); subsystem_register(&bus->subsys); - snprintf(bus->devices.kobj.name,KOBJ_NAME_LEN,"devices"); + strlcpy(bus->devices.kobj.name, "devices", KOBJ_NAME_LEN); bus->devices.subsys = &bus->subsys; kset_register(&bus->devices); - snprintf(bus->drivers.kobj.name,KOBJ_NAME_LEN,"drivers"); + strlcpy(bus->drivers.kobj.name, "drivers", KOBJ_NAME_LEN); bus->drivers.subsys = &bus->subsys; bus->drivers.ktype = &ktype_driver; kset_register(&bus->drivers); ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Resend [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-25 3:52 ` Linus Torvalds ` (3 preceding siblings ...) 2003-05-25 16:41 ` Ben Collins @ 2003-07-11 9:50 ` Rogier Wolff 4 siblings, 0 replies; 19+ messages in thread From: Rogier Wolff @ 2003-07-11 9:50 UTC (permalink / raw) To: Linus Torvalds; +Cc: Ben Collins, Patrick Mochel, linux-kernel On Sat, May 24, 2003 at 08:52:53PM -0700, Linus Torvalds wrote: > How about just adding a sane > > int copy_string(char *dest, const char *src, int len) > { > int size; > > if (!len) > return 0; > size = strlen(src); > if (size >= len) > size = len-1; > memcpy(dest, src, size); > dest[size] = '\0'; > return size; > } Just catching up... Most people will think: "But that's not efficient!" he first determines the size using strlen, and only then does he start a memcpy. In fact most modern processors priming the cache and then doing the copy is noticably faster or just a teeny little bit slower. Roger. -- ** R.E.Wolff@BitWizard.nl ** http://www.BitWizard.nl/ ** +31-15-2600998 ** *-- BitWizard writes Linux device drivers for any device you may have! --* * I didn't say it was your fault. I said I was going to blame it on you. * ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Resend [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE 2003-05-25 0:07 ` Ben Collins 2003-05-25 3:52 ` Linus Torvalds @ 2003-05-25 8:02 ` Russell King 1 sibling, 0 replies; 19+ messages in thread From: Russell King @ 2003-05-25 8:02 UTC (permalink / raw) To: Ben Collins; +Cc: Patrick Mochel, Linus Torvalds, linux-kernel On Sat, May 24, 2003 at 08:07:01PM -0400, Ben Collins wrote: > Given that the problem with KOBJ_NAME_LEN == 20 affecting one snd driver > has so far only been explained as a compiler bug, can I suggest this > patch be applied? No one has confirmed that it is a compiler bug yet. We only suspect that something in the yamaha driver is getting miscompiled. We don't know what, we don't know how, we don't know why. -- Russell King (rmk@arm.linux.org.uk) The developer of ARM Linux http://www.arm.linux.org.uk/personal/aboutme.html ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2003-07-11 9:35 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2003-05-13 6:26 [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE Ben Collins 2003-05-13 7:10 ` Christoph Hellwig 2003-05-13 7:14 ` Ben Collins 2003-05-13 15:08 ` Patrick Mochel 2003-05-16 0:20 ` Resend " Ben Collins 2003-05-16 18:43 ` Felipe Alfaro Solana 2003-05-25 0:07 ` Ben Collins 2003-05-25 3:52 ` Linus Torvalds 2003-05-25 3:10 ` Ben Collins 2003-05-25 12:03 ` Adam Sampson 2003-05-25 17:10 ` Linus Torvalds 2003-05-25 16:40 ` Ben Collins 2003-05-25 15:51 ` Matt Mackall 2003-05-25 17:25 ` Riley Williams 2003-05-25 18:13 ` Valdis.Kletnieks 2003-05-25 23:42 ` Matt Mackall 2003-05-25 16:41 ` Ben Collins 2003-07-11 9:50 ` Rogier Wolff 2003-05-25 8:02 ` Russell King
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).