Check if the alloc_fn and free_fn are not NULL. The caller generally
ensures that alloc_fn and free_fn are valid. It would not harm
to check. This makes the checking in mempool_create() more complete.
--- mempool.c.org Fri Feb 22 12:00:58 2002
+++ mempool.c Fri Feb 22 12:01:13 2002
@@ -35,7 +35,7 @@
int i;
pool = kmalloc(sizeof(*pool), GFP_KERNEL);
- if (!pool)
+ if (!pool || !alloc_fn || !free_fn)
return NULL;
memset(pool, 0, sizeof(*pool));
Enjoy,
Balbir
_________________________________________________________________
Send and receive Hotmail on your mobile device: http://mobile.msn.com
>Check if the alloc_fn and free_fn are not NULL. The caller generally
>ensures that alloc_fn and free_fn are valid. It would not harm
>to check. This makes the checking in mempool_create() more complete.
>
>
>--- mempool.c.org Fri Feb 22 12:00:58 2002
>+++ mempool.c Fri Feb 22 12:01:13 2002
>@@ -35,7 +35,7 @@
> int i;
>
> pool = kmalloc(sizeof(*pool), GFP_KERNEL);
>- if (!pool)
>+ if (!pool || !alloc_fn || !free_fn)
> return NULL;
> memset(pool, 0, sizeof(*pool));
>
A successful allocation with alloc_fn or free_fn equal to NULL
would return NULL, without freeing pool. => This check would
leak memory? Wouldn't it be better to check for !alloc_fn || !free_fn
before the kmalloc()
--
Marcus Alanen
[email protected]
You are absoultely correct. The correct patch is
--- mempool.c.org Fri Feb 22 12:00:58 2002
+++ mempool.c Fri Feb 22 15:01:02 2002
@@ -34,6 +34,9 @@
mempool_t *pool;
int i;
+ if (!alloc_fn || !free_fn)
+ return NULL;
+
pool = kmalloc(sizeof(*pool), GFP_KERNEL);
if (!pool)
return NULL;
Balbir Singh.
>From: Marcus Alanen <[email protected]>
>To: [email protected], [email protected]
>Subject: Re: [PATCH] Trivial patch against mempool
>Date: Fri, 22 Feb 2002 23:40:43 +0200
>
> >Check if the alloc_fn and free_fn are not NULL. The caller generally
> >ensures that alloc_fn and free_fn are valid. It would not harm
> >to check. This makes the checking in mempool_create() more complete.
> >
> >
> >--- mempool.c.org Fri Feb 22 12:00:58 2002
> >+++ mempool.c Fri Feb 22 12:01:13 2002
> >@@ -35,7 +35,7 @@
> > int i;
> >
> > pool = kmalloc(sizeof(*pool), GFP_KERNEL);
> >- if (!pool)
> >+ if (!pool || !alloc_fn || !free_fn)
> > return NULL;
> > memset(pool, 0, sizeof(*pool));
> >
>
>A successful allocation with alloc_fn or free_fn equal to NULL
>would return NULL, without freeing pool. => This check would
>leak memory? Wouldn't it be better to check for !alloc_fn || !free_fn
>before the kmalloc()
>
>
>--
>Marcus Alanen
>[email protected]
_________________________________________________________________
Send and receive Hotmail on your mobile device: http://mobile.msn.com
On Fri, Feb 22, 2002 at 12:28:14PM -0800, Balbir Singh wrote:
> Check if the alloc_fn and free_fn are not NULL. The caller generally
> ensures that alloc_fn and free_fn are valid. It would not harm
> to check. This makes the checking in mempool_create() more complete.
Rather than leak memory in that case, why not just BUG_ON null
function pointers so that people know what code is at fault?
-ben
That is a good suggestion too, I have redone the patch so
that the alloc_fn and free_fn are checked before calling
kmalloc(). I think the BUG_ON is also a good solution
I have the following now
1.
--- mempool.c.org Fri Feb 22 12:00:58 2002
+++ mempool.c Fri Feb 22 17:26:02 2002
@@ -34,6 +34,9 @@
mempool_t *pool;
int i;
+ BUG_ON(!alloc_fn);
+ BUG_ON(!free_fn);
+
pool = kmalloc(sizeof(*pool), GFP_KERNEL);
if (!pool)
return NULL;
2.
--- mempool.c.org Fri Feb 22 12:00:58 2002
+++ mempool.c Fri Feb 22 17:38:52 2002
@@ -34,6 +34,8 @@
mempool_t *pool;
int i;
+ BUG_ON(!(alloc_fn && free_fn));
+
pool = kmalloc(sizeof(*pool), GFP_KERNEL);
if (!pool)
return NULL;
I think (1) is more readable, what do you say?
Balbir
>From: Benjamin LaHaise <[email protected]>
>To: Balbir Singh <[email protected]>
>CC: [email protected]
>Subject: Re: [PATCH] Trivial patch against mempool
>Date: Fri, 22 Feb 2002 19:02:56 -0500
>
>On Fri, Feb 22, 2002 at 12:28:14PM -0800, Balbir Singh wrote:
> > Check if the alloc_fn and free_fn are not NULL. The caller generally
> > ensures that alloc_fn and free_fn are valid. It would not harm
> > to check. This makes the checking in mempool_create() more complete.
>
>Rather than leak memory in that case, why not just BUG_ON null
>function pointers so that people know what code is at fault?
>
> -ben
_________________________________________________________________
MSN Photos is the easiest way to share and print your photos:
http://photos.msn.com/support/worldwide.aspx