[RFC][PATCH] kmalloc + memset(foo, 0, bar) = kmalloc0

[RFC][PATCH] kmalloc + memset(foo, 0, bar) = kmalloc0

[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));

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

--- linux-2.6.0-test5-bk1/include/linux/slab.h	2003-09-11 15:19:40.000000000 +0200
+++ linux-2.6.0-test5-bk1-caliban/include/linux/slab.h	2003-09-11 15:22:56.000000000 +0200
@@ -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 *);
 

Re: [RFC][PATCH] kmalloc + memset(foo, 0, bar) = kmalloc0

[viro]

On Thu, Sep 11, 2003 at 03:40:58PM +0200, Rolf Eike Beer wrote:
> 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...

> 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().

Re: [RFC][PATCH] kmalloc + memset(foo, 0, bar) = kmalloc0

[Breno Silva]

Do correct kmalloc+memset in new funcion.

Breno
----- Original Message -----
From: "Rolf Eike Beer" <eike-kernel@sf-tec.de>
To: <linux-kernel@vger.kernel.org>
Sent: Thursday, September 11, 2003 10:40 AM
Subject: [RFC][PATCH] 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
>
> --- linux-2.6.0-test5-bk1/include/linux/slab.h 2003-09-11
15:19:40.000000000 +0200
> +++ linux-2.6.0-test5-bk1-caliban/include/linux/slab.h 2003-09-11
15:22:56.000000000 +0200
> @@ -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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC][PATCH] kmalloc + memset(foo, 0, bar) = kmalloc0

[Rolf Eike Beer]

Am Donnerstag, 11. September 2003 15:45 schrieben Sie:
> On Thu, Sep 11, 2003 at 03:40:58PM +0200, Rolf Eike Beer wrote:
> > 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...

Eek, yes. Typo from me.

> > 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

Re: [RFC][PATCH] kmalloc + memset(foo, 0, bar) = kmalloc0

[Jamie Lokier]

viro@parcelfarce.linux.theplanet.co.uk wrote:
> Bad choice of name - too easy to confuse with kmalloc().

kmalloc_and_zero() would be much clearer.

It opens the way for a pool of pre-zeroed pages, too.  We know that
improves preformance on some architectures.

-- Jamie

Re: [RFC][PATCH] kmalloc + memset(foo, 0, bar) = kmalloc0

[J.A. Magallon]

On 09.11, Jamie Lokier wrote:
> viro@parcelfarce.linux.theplanet.co.uk wrote:
> > Bad choice of name - too easy to confuse with kmalloc().
> 
> kmalloc_and_zero() would be much clearer.
> 

Why not kcalloc() ?

-- 
J.A. Magallon <jamagallon@able.es>      \                 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))

Re: [RFC][PATCH] kmalloc + memset(foo, 0, bar) = kmalloc0

[Alan Cox]

On Iau, 2003-09-11 at 22:58, J.A. Magallon wrote:
> On 09.11, Jamie Lokier wrote:
> > viro@parcelfarce.linux.theplanet.co.uk 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

Re: [RFC][PATCH] kmalloc + memset(foo, 0, bar) = kmalloc0

[J.A. Magallon]

On 09.12, Alan Cox wrote:
> On Iau, 2003-09-11 at 22:58, J.A. Magallon wrote:
> > On 09.11, Jamie Lokier wrote:
> > > viro@parcelfarce.linux.theplanet.co.uk 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
> 

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 ;).

-- 
J.A. Magallon <jamagallon@able.es>      \                 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))