* [PATCH 1/4] drivers/block/xen-blkback/blkback.c: take size of pointed value, not pointer
@ 2011-09-16 6:57 Julia Lawall
2011-09-16 7:38 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: Julia Lawall @ 2011-09-16 6:57 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: kernel-janitors, Ian Campbell, Jan Beulich, linux-kernel
From: Julia Lawall <julia@diku.dk>
Sizeof a pointer-typed expression returns the size of the pointer, not that
of the pointed data.
The semantic patch that fixes this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
expression *e;
type T;
identifier f;
@@
f(...,(T)e,...,
-sizeof(e)
+sizeof(*e)
,...)
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
drivers/block/xen-blkback/blkback.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff -u -p a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -790,7 +790,7 @@ static int __init xen_blkif_init(void)
if (rc)
goto failed_init;
- memset(blkbk->pending_reqs, 0, sizeof(blkbk->pending_reqs));
+ memset(blkbk->pending_reqs, 0, sizeof(*blkbk->pending_reqs));
INIT_LIST_HEAD(&blkbk->pending_free);
spin_lock_init(&blkbk->pending_free_lock);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/4] drivers/block/xen-blkback/blkback.c: take size of pointed value, not pointer
2011-09-16 6:57 [PATCH 1/4] drivers/block/xen-blkback/blkback.c: take size of pointed value, not pointer Julia Lawall
@ 2011-09-16 7:38 ` Jan Beulich
2011-09-16 8:08 ` Julia Lawall
0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2011-09-16 7:38 UTC (permalink / raw)
To: Julia Lawall, Konrad Rzeszutek Wilk
Cc: Ian Campbell, kernel-janitors, linux-kernel
>>> On 16.09.11 at 08:57, Julia Lawall <julia@diku.dk> wrote:
> From: Julia Lawall <julia@diku.dk>
>
> Sizeof a pointer-typed expression returns the size of the pointer, not that
> of the pointed data.
>
> The semantic patch that fixes this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression *e;
> type T;
> identifier f;
> @@
>
> f(...,(T)e,...,
> -sizeof(e)
> +sizeof(*e)
> ,...)
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
>
> ---
> drivers/block/xen-blkback/blkback.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff -u -p a/drivers/block/xen-blkback/blkback.c
> b/drivers/block/xen-blkback/blkback.c
> --- a/drivers/block/xen-blkback/blkback.c
> +++ b/drivers/block/xen-blkback/blkback.c
> @@ -790,7 +790,7 @@ static int __init xen_blkif_init(void)
> if (rc)
> goto failed_init;
>
> - memset(blkbk->pending_reqs, 0, sizeof(blkbk->pending_reqs));
> + memset(blkbk->pending_reqs, 0, sizeof(*blkbk->pending_reqs));
>
> INIT_LIST_HEAD(&blkbk->pending_free);
> spin_lock_init(&blkbk->pending_free_lock);
I think a better fix for this is to use kzalloc() properly here:
Subject: xen-blkback: use kzalloc() in favor of kmalloc()+memset()
This fixes the problem of three of those four memset()-s having
improper size arguments passed: Sizeof a pointer-typed expression
returns the size of the pointer, not that of the pointed to data.
It also reverts using kmalloc() instead of kzalloc() for the allocation
of the pending grant handles array, as that array gets fully
initialized in a subsequent loop.
Reported-by: Julia Lawall <julia@diku.dk>
Signed-off-by: Jan Beulich <jbeulich@novell.com>
---
drivers/block/xen-blkback/blkback.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
--- 3.1-rc6/drivers/block/xen-blkback/blkback.c
+++ 3.1-rc6-xen-blkback-kzalloc/drivers/block/xen-blkback/blkback.c
@@ -765,9 +765,9 @@ static int __init xen_blkif_init(void)
mmap_pages = xen_blkif_reqs * BLKIF_MAX_SEGMENTS_PER_REQUEST;
- blkbk->pending_reqs = kmalloc(sizeof(blkbk->pending_reqs[0]) *
+ blkbk->pending_reqs = kzalloc(sizeof(blkbk->pending_reqs[0]) *
xen_blkif_reqs, GFP_KERNEL);
- blkbk->pending_grant_handles = kzalloc(sizeof(blkbk->pending_grant_handles[0]) *
+ blkbk->pending_grant_handles = kmalloc(sizeof(blkbk->pending_grant_handles[0]) *
mmap_pages, GFP_KERNEL);
blkbk->pending_pages = kzalloc(sizeof(blkbk->pending_pages[0]) *
mmap_pages, GFP_KERNEL);
@@ -790,8 +790,6 @@ static int __init xen_blkif_init(void)
if (rc)
goto failed_init;
- memset(blkbk->pending_reqs, 0, sizeof(blkbk->pending_reqs));
-
INIT_LIST_HEAD(&blkbk->pending_free);
spin_lock_init(&blkbk->pending_free_lock);
init_waitqueue_head(&blkbk->pending_free_wq);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/4] drivers/block/xen-blkback/blkback.c: take size of pointed value, not pointer
2011-09-16 7:38 ` Jan Beulich
@ 2011-09-16 8:08 ` Julia Lawall
0 siblings, 0 replies; 3+ messages in thread
From: Julia Lawall @ 2011-09-16 8:08 UTC (permalink / raw)
To: Jan Beulich
Cc: Konrad Rzeszutek Wilk, Ian Campbell, kernel-janitors,
linux-kernel
> I think a better fix for this is to use kzalloc() properly here:
Indeed, it seems like a much better idea :)
julia
> Subject: xen-blkback: use kzalloc() in favor of kmalloc()+memset()
>
> This fixes the problem of three of those four memset()-s having
> improper size arguments passed: Sizeof a pointer-typed expression
> returns the size of the pointer, not that of the pointed to data.
>
> It also reverts using kmalloc() instead of kzalloc() for the allocation
> of the pending grant handles array, as that array gets fully
> initialized in a subsequent loop.
>
> Reported-by: Julia Lawall <julia@diku.dk>
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>
> ---
> drivers/block/xen-blkback/blkback.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> --- 3.1-rc6/drivers/block/xen-blkback/blkback.c
> +++ 3.1-rc6-xen-blkback-kzalloc/drivers/block/xen-blkback/blkback.c
> @@ -765,9 +765,9 @@ static int __init xen_blkif_init(void)
>
> mmap_pages = xen_blkif_reqs * BLKIF_MAX_SEGMENTS_PER_REQUEST;
>
> - blkbk->pending_reqs = kmalloc(sizeof(blkbk->pending_reqs[0]) *
> + blkbk->pending_reqs = kzalloc(sizeof(blkbk->pending_reqs[0]) *
> xen_blkif_reqs, GFP_KERNEL);
> - blkbk->pending_grant_handles = kzalloc(sizeof(blkbk->pending_grant_handles[0]) *
> + blkbk->pending_grant_handles = kmalloc(sizeof(blkbk->pending_grant_handles[0]) *
> mmap_pages, GFP_KERNEL);
> blkbk->pending_pages = kzalloc(sizeof(blkbk->pending_pages[0]) *
> mmap_pages, GFP_KERNEL);
> @@ -790,8 +790,6 @@ static int __init xen_blkif_init(void)
> if (rc)
> goto failed_init;
>
> - memset(blkbk->pending_reqs, 0, sizeof(blkbk->pending_reqs));
> -
> INIT_LIST_HEAD(&blkbk->pending_free);
> spin_lock_init(&blkbk->pending_free_lock);
> init_waitqueue_head(&blkbk->pending_free_wq);
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-09-16 8:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-16 6:57 [PATCH 1/4] drivers/block/xen-blkback/blkback.c: take size of pointed value, not pointer Julia Lawall
2011-09-16 7:38 ` Jan Beulich
2011-09-16 8:08 ` Julia Lawall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox