Skip to contentSkip to conversation

kmalloc + memset(foo, 0, bar) = kmalloc0

RFC PATCH In review Rolf Eike Beer 6 participants 8 messages Sep 11, 2003 Slab allocator

lore
Share & tools
lore

Rolf Eike Beer

author
#0 lore raw

RFC PATCH 1/1 kmalloc + memset(foo, 0, bar) = kmalloc0

Hi,

a (very) simple grep in drivers/ showed more than 300 matches of code like this:

foo = kmalloc(bar, baz);
if (! foo)

	return -ENOMEM;

memset(foo, 0, sizeof(foo));

Why not add a small inlined function doing the memset for us and reducing the code to

foo = kmalloc0(bar, baz);
if (! foo)

	return -ENOMEM;

Eike

include/linux/slab.h +11 -0
@@ -14,6 +14,7 @@
 #include	<linux/config.h>	/* kmalloc_sizes.h needs CONFIG_ options */
 #include	<linux/gfp.h>
 #include	<linux/types.h>
+#include	<linux/string.h>	/* for memset */
 #include	<asm/page.h>		/* kmalloc_sizes.h needs PAGE_SIZE */
 #include	<asm/cache.h>		/* kmalloc_sizes.h needs L1_CACHE_BYTES */
 

@@ -97,6 +98,16 @@
 	return __kmalloc(size, flags);
 }
 
+static inline void *kmalloc0(size_t size, int flags)
+{
+	void *res = kmalloc(size, flags);
+
+	if (res != NULL)
+		memset(res, 0, size);
+
+	return res;
+}
+
 extern void kfree(const void *);
 extern unsigned int ksize(const void *);
#1 lore raw
Quoted 9 lines · Rolf Eike Beer

Hi,

a (very) simple grep in drivers/ showed more than 300 matches of code like this:

foo = kmalloc(bar, baz);
if (! foo)

	return -ENOMEM;

memset(foo, 0, sizeof(foo));

Erm. It would better *not* be there in such amounts - sizeof(foo) would be a size of pointer...

Quoted 6 lines

Why not add a small inlined function doing the memset for us and reducing the code to

foo = kmalloc0(bar, baz);
if (! foo)

	return -ENOMEM;

Bad choice of name - too easy to confuse with kmalloc().

author
#2 lore raw

Am Donnerstag, 11. September 2003 15:45 schrieben Sie:

Quoted 6 lines · Rolf Eike Beer
Quoted 5 lines

Hi,

a (very) simple grep in drivers/ showed more than 300 matches of code like this:

Quoted 7 lines
Quoted 4 lines

foo = kmalloc(bar, baz);
if (! foo)

	return -ENOMEM;

memset(foo, 0, sizeof(foo));

Erm. It would better *not* be there in such amounts - sizeof(foo) would be a size of pointer...

Eek, yes. Typo from me.

Quoted 8 lines
Quoted 6 lines

Why not add a small inlined function doing the memset for us and reducing the code to

foo = kmalloc0(bar, baz);
if (! foo)

	return -ENOMEM;

Bad choice of name - too easy to confuse with kmalloc().

Yes, maybe. But don't expect more innovations from me today, it's someone else's turn ;)

Eike

#4 lore raw
Quoted 5 lines · Jamie Lokier
Quoted 1 line · [email protected] wrote:

Bad choice of name - too easy to confuse with kmalloc().

kmalloc_and_zero() would be much clearer.

Why not kcalloc() ?

Signature · 4 lines
J.A. Magallon <[email protected]>      \                 Software is like sex:
werewolf.able.es                         \           It's better when it's free
Mandrake Linux release 9.2 (Cooker) for i586
Linux 2.4.23-pre2-jam1m (gcc 3.3.1 (Mandrake Linux 9.2 3.3.1-1mdk))
#5 lore raw
Quoted 8 lines · J.A. Magallon
Quoted 5 lines · Jamie Lokier
Quoted 1 line · [email protected] wrote:

Bad choice of name - too easy to confuse with kmalloc().

kmalloc_and_zero() would be much clearer.

Why not kcalloc() ?

A kcalloc that also checked for maths overflows would probably help avoid various errors and checks against ~0/sizeof(n) in drivers we have now

#6 lore raw
Quoted 5 lines · Alan Cox
Quoted 4 lines · J.A. Magallon
Quoted 3 lines · Jamie Lokier
Quoted 1 line · [email protected] wrote:

Bad choice of name - too easy to confuse with kmalloc().

Quoted 9 lines
Quoted 4 lines
Quoted 2 lines

kmalloc_and_zero() would be much clearer.

Why not kcalloc() ?

A kcalloc that also checked for maths overflows would probably help avoid various errors and checks against ~0/sizeof(n) in drivers we have now

Some time ago when I tried to do some wrappers, I choosed something like this (k-prefixed here for kernel...):

void* kalloc(size_t size)
void* kvalloc(size_t size,size_t num)
void* kallocz(size_t size) // or kalloc_z, clearer... void* kvallocz(size_t size,size_t num) // kvalloc_z void* kfree(void*)

And if you don't hate too much C++

#define knew(mytype) (mytype*)kalloc(sizeof(mytype)) #define kvnew(mytype,n) (mytype*)kvalloc(sizeof(mytype),n)

and so on...
I can tell for sure they clarify the code very much... and you don't forget the NULL check anymore ;).

Signature · 4 lines
J.A. Magallon <[email protected]>      \                 Software is like sex:
werewolf.able.es                         \           It's better when it's free
Mandrake Linux release 9.2 (Cooker) for i586
Linux 2.4.23-pre2-jam1m (gcc 3.3.1 (Mandrake Linux 9.2 3.3.1-1mdk))
#7 lore raw
In reply to Rolf Eike Beer

Do correct kmalloc+memset in new funcion.

Breno
----- Original Message -----
From: "Rolf Eike Beer" <[email protected]>
To: <[email protected]>
Sent: Thursday, September 11, 2003 10:40 AM
Subject: [RFC][PATCH] kmalloc + memset(foo, 0, bar) = kmalloc0

Quoted 13 lines

Hi,

a (very) simple grep in drivers/ showed more than 300 matches of code like this:

foo = kmalloc(bar, baz);
if (! foo)
return -ENOMEM;
memset(foo, 0, sizeof(foo));

Why not add a small inlined function doing the memset for us and reducing the code to

Quoted 7 lines

foo = kmalloc0(bar, baz);
if (! foo)
return -ENOMEM;

Eike

--- linux-2.6.0-test5-bk1/include/linux/slab.h 2003-09-11

15:19:40.000000000 +0200

Quoted 1 line

+++ linux-2.6.0-test5-bk1-caliban/include/linux/slab.h 2003-09-11

15:22:56.000000000 +0200

Quoted 30 lines
include/linux/slab.h +11 -1
@@ -14,6 +14,7 @@
 #include <linux/config.h> /* kmalloc_sizes.h needs CONFIG_ options */
 #include <linux/gfp.h>
 #include <linux/types.h>
+#include <linux/string.h> /* for memset */
 #include <asm/page.h> /* kmalloc_sizes.h needs PAGE_SIZE */
 #include <asm/cache.h> /* kmalloc_sizes.h needs L1_CACHE_BYTES */
 

@@ -97,6 +98,16 @@
  return __kmalloc(size, flags);
 }
 
+static inline void *kmalloc0(size_t size, int flags)
+{
+ void *res = kmalloc(size, flags);
+
+ if (res != NULL)
+ memset(res, 0, size);
+
+ return res;
+}
+
 extern void kfree(const void *);
 extern unsigned int ksize(const void *);
 
-

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