One of idiomatic ways to duplicate a region of memory is
dst = kmalloc(len, GFP_KERNEL);
if (!dst)
return -ENOMEM;
memcpy(dst, src, len);
which is neat code except a programmer needs to write size twice. Which
sometimes leads to mistakes. If len passed to kmalloc is smaller that len
passed to memcpy, it's straight overwrite-beyond-end. If len passed to
memcpy is smaller than len passed to kmalloc, it's either a) legit
behaviour ;-), or b) cloned buffer will contain garbage in second half.
[TODO: list of such bugs in Linux, other kernels, userspace]
If programmer is given only one place to play with lengths, I believe, such
mistakes could be avoided.
With kmemdup, the snippet above will be rewritten as:
dst = kmemdup(src, len, GFP_KERNEL);
if (!dst)
return -ENOMEM;
This also leads to smaller code (kzalloc effect).
Not tested yet, this is for semantics commentary.
The plan is to
a) merge kmemdup
b) get some users (patch #2)
c) stick kmemdup conversion into -kj TODO
d) grep for memcpy and caaaarefully convert the rest. Promise to be a bastard.
----------------
e) try to get memdup(3) somehow, but I'm probably dreaming...
P.S.: No idea why kstrdup() and kzalloc() use _____________kmalloc(),
but I followed the crowd.
---
include/linux/string.h | 1 +
mm/util.c | 18 ++++++++++++++++++
2 files changed, 19 insertions(+)
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -99,6 +99,7 @@ extern void * memchr(const void *,int,__
#endif
extern char *kstrdup(const char *s, gfp_t gfp);
+extern void *kmemdup(const void *src, size_t len, gfp_t gfp);
#ifdef __cplusplus
}
--- a/mm/util.c
+++ b/mm/util.c
@@ -40,6 +40,24 @@ char *kstrdup(const char *s, gfp_t gfp)
}
EXPORT_SYMBOL(kstrdup);
+/**
+ * kmemdup - duplicate region of memory
+ *
+ * @src: memory region to duplicate
+ * @len: memory region length
+ * @gfp: GFP mask to use
+ */
+void *kmemdup(const void *src, size_t len, gfp_t gfp)
+{
+ void *p;
+
+ p = ____kmalloc(len, gfp);
+ if (p)
+ memcpy(p, src, len);
+ return p;
+}
+EXPORT_SYMBOL(kmemdup);
+
/*
* strndup_user - duplicate an existing string from user space
*
On Sat, 9 Sep 2006 05:35:55 +0400
Alexey Dobriyan <[email protected]> wrote:
> P.S.: No idea why kstrdup() and kzalloc() use _____________kmalloc(),
It's all to do with slab debugging and __kmalloc_track_caller(): we want
to record the _caller_ of kstrdup() within the slab object rather than kstrdup()
itself.
On 9/9/06, Alexey Dobriyan <[email protected]> wrote:
> +/**
> + * kmemdup - duplicate region of memory
> + *
> + * @src: memory region to duplicate
> + * @len: memory region length
> + * @gfp: GFP mask to use
> + */
> +void *kmemdup(const void *src, size_t len, gfp_t gfp)
> +{
> + void *p;
> +
> + p = ____kmalloc(len, gfp);
> + if (p)
> + memcpy(p, src, len);
> + return p;
> +}
> +EXPORT_SYMBOL(kmemdup);
Assuming there are enough callers that can be converted to justify
this, looks good to me.
On Sat, 9 Sep 2006 05:35:55 +0400 Alexey Dobriyan wrote:
> Not tested yet, this is for semantics commentary.
>
> include/linux/string.h | 1 +
> mm/util.c | 18 ++++++++++++++++++
> 2 files changed, 19 insertions(+)
>
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -40,6 +40,24 @@ char *kstrdup(const char *s, gfp_t gfp)
> }
> EXPORT_SYMBOL(kstrdup);
>
> +/**
> + * kmemdup - duplicate region of memory
> + *
No blank line here, please. kernel-doc "language" doesn't allow
that. Hopefully that will be fixed someday.
> + * @src: memory region to duplicate
> + * @len: memory region length
> + * @gfp: GFP mask to use
> + */
> +void *kmemdup(const void *src, size_t len, gfp_t gfp)
> +{
> + void *p;
> +
> + p = ____kmalloc(len, gfp);
> + if (p)
> + memcpy(p, src, len);
> + return p;
> +}
> +EXPORT_SYMBOL(kmemdup);
---
~Randy