* Re: [KJ] [PATCH] cris/arch-v32: Remove unnecessary cast of return
2007-06-29 16:12 [KJ] [PATCH] cris/arch-v32: Remove unnecessary cast of return value Suresh Jayaraman
@ 2007-07-03 12:28 ` walter harms
2007-07-04 5:02 ` [KJ] [PATCH] cris/arch-v32: Remove unnecessary cast of Suresh Jayaraman
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: walter harms @ 2007-07-03 12:28 UTC (permalink / raw)
To: kernel-janitors
Suresh Jayaraman wrote:
> Remove unnecessary cast of return value of kmalloc() in
> cris/arch-v32/mm/intmem.c
>
> Signed-off-by: Suresh Jayaraman <sjayaraman@novell.com>
> ---
>
> diff --git a/arch/cris/arch-v32/mm/intmem.c b/arch/cris/arch-v32/mm/intmem.c
> index 41ee7f7..d1e0bf4 100644
> --- a/arch/cris/arch-v32/mm/intmem.c
> +++ b/arch/cris/arch-v32/mm/intmem.c
> @@ -27,8 +27,8 @@ static void crisv32_intmem_init(void)
> {
> static int initiated = 0;
> if (!initiated) {
> - struct intmem_allocation* alloc > - (struct intmem_allocation*)kmalloc(sizeof *alloc, GFP_KERNEL);
> + struct intmem_allocation *alloc = kmalloc(sizeof *alloc,
> + GFP_KERNEL);
IMHO every kmalloc() should check if it worked at all
if (!alloc) {
kprintf("no memory panic !\n");
return;
}
re,
wh
> INIT_LIST_HEAD(&intmem_allocations);
> intmem_virtual = ioremap(MEM_INTMEM_START, MEM_INTMEM_SIZE);
> initiated = 1;
> @@ -55,8 +55,7 @@ void* crisv32_intmem_alloc(unsigned size, unsigned align)
> if (allocation->status = STATUS_FREE &&
> allocation->size >= size + alignment) {
> if (allocation->size > size + alignment) {
> - struct intmem_allocation* alloc > - (struct intmem_allocation*)
> + struct intmem_allocation *alloc > kmalloc(sizeof *alloc, GFP_ATOMIC);
> alloc->status = STATUS_FREE;
> alloc->size = allocation->size - size - alignment;
> @@ -65,8 +64,7 @@ void* crisv32_intmem_alloc(unsigned size, unsigned align)
>
> if (alignment) {
> struct intmem_allocation* tmp;
> - tmp = (struct intmem_allocation*)
> - kmalloc(sizeof *tmp, GFP_ATOMIC);
> + tmp = kmalloc(sizeof *tmp, GFP_ATOMIC);
> tmp->offset = allocation->offset;
> tmp->size = alignment;
> tmp->status = STATUS_FREE;
>
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [KJ] [PATCH] cris/arch-v32: Remove unnecessary cast of
2007-06-29 16:12 [KJ] [PATCH] cris/arch-v32: Remove unnecessary cast of return value Suresh Jayaraman
2007-07-03 12:28 ` [KJ] [PATCH] cris/arch-v32: Remove unnecessary cast of return walter harms
@ 2007-07-04 5:02 ` Suresh Jayaraman
2007-07-04 9:01 ` [KJ] [PATCH] cris/arch-v32: Remove unnecessary cast of return walter harms
2007-07-04 10:43 ` [KJ] [PATCH] cris/arch-v32: Remove unnecessary cast of Suresh Jayaraman
3 siblings, 0 replies; 5+ messages in thread
From: Suresh Jayaraman @ 2007-07-04 5:02 UTC (permalink / raw)
To: kernel-janitors
On Tue, 2007-07-03 at 12:32 +0000, walter harms wrote:
>
> Suresh Jayaraman wrote:
> > Remove unnecessary cast of return value of kmalloc() in
> > cris/arch-v32/mm/intmem.c
> >
> > Signed-off-by: Suresh Jayaraman <sjayaraman@novell.com>
> > ---
> >
> > diff --git a/arch/cris/arch-v32/mm/intmem.c b/arch/cris/arch-v32/mm/intmem.c
> > index 41ee7f7..d1e0bf4 100644
> > --- a/arch/cris/arch-v32/mm/intmem.c
> > +++ b/arch/cris/arch-v32/mm/intmem.c
> > @@ -27,8 +27,8 @@ static void crisv32_intmem_init(void)
> > {
> > static int initiated = 0;
> > if (!initiated) {
> > - struct intmem_allocation* alloc > > - (struct intmem_allocation*)kmalloc(sizeof *alloc, GFP_KERNEL);
> > + struct intmem_allocation *alloc = kmalloc(sizeof *alloc,
> > + GFP_KERNEL);
>
> IMHO every kmalloc() should check if it worked at all
>
> if (!alloc) {
> kprintf("no memory panic !\n");
> return;
> }
If we all agree that kmalloc() checks should be done everwhere, this
probably can be a separate Kernel Janitor task.
--
Suresh Jayaraman
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [KJ] [PATCH] cris/arch-v32: Remove unnecessary cast of return
2007-06-29 16:12 [KJ] [PATCH] cris/arch-v32: Remove unnecessary cast of return value Suresh Jayaraman
2007-07-03 12:28 ` [KJ] [PATCH] cris/arch-v32: Remove unnecessary cast of return walter harms
2007-07-04 5:02 ` [KJ] [PATCH] cris/arch-v32: Remove unnecessary cast of Suresh Jayaraman
@ 2007-07-04 9:01 ` walter harms
2007-07-04 10:43 ` [KJ] [PATCH] cris/arch-v32: Remove unnecessary cast of Suresh Jayaraman
3 siblings, 0 replies; 5+ messages in thread
From: walter harms @ 2007-07-04 9:01 UTC (permalink / raw)
To: kernel-janitors
Suresh Jayaraman wrote:
> On Tue, 2007-07-03 at 12:32 +0000, walter harms wrote:
>> Suresh Jayaraman wrote:
>>> Remove unnecessary cast of return value of kmalloc() in
>>> cris/arch-v32/mm/intmem.c
>>>
>>> Signed-off-by: Suresh Jayaraman <sjayaraman@novell.com>
>>> ---
>>>
>>> diff --git a/arch/cris/arch-v32/mm/intmem.c b/arch/cris/arch-v32/mm/intmem.c
>>> index 41ee7f7..d1e0bf4 100644
>>> --- a/arch/cris/arch-v32/mm/intmem.c
>>> +++ b/arch/cris/arch-v32/mm/intmem.c
>>> @@ -27,8 +27,8 @@ static void crisv32_intmem_init(void)
>>> {
>>> static int initiated = 0;
>>> if (!initiated) {
>>> - struct intmem_allocation* alloc >>> - (struct intmem_allocation*)kmalloc(sizeof *alloc, GFP_KERNEL);
>>> + struct intmem_allocation *alloc = kmalloc(sizeof *alloc,
>>> + GFP_KERNEL);
>> IMHO every kmalloc() should check if it worked at all
>>
>> if (!alloc) {
>> kprintf("no memory panic !\n");
>> return;
>
>> }
>
> If we all agree that kmalloc() checks should be done everwhere, this
> probably can be a separate Kernel Janitor task.
>
you are right, normaly it would better to have two patches. IMHO for this special case
(since removing a case is small change) you add the check.
The important point is to look not only for "the task" - like removing the cast - but also
for other errors. i may take some time before the next will check that file again.
re,
wh
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [KJ] [PATCH] cris/arch-v32: Remove unnecessary cast of
2007-06-29 16:12 [KJ] [PATCH] cris/arch-v32: Remove unnecessary cast of return value Suresh Jayaraman
` (2 preceding siblings ...)
2007-07-04 9:01 ` [KJ] [PATCH] cris/arch-v32: Remove unnecessary cast of return walter harms
@ 2007-07-04 10:43 ` Suresh Jayaraman
3 siblings, 0 replies; 5+ messages in thread
From: Suresh Jayaraman @ 2007-07-04 10:43 UTC (permalink / raw)
To: kernel-janitors
On Wed, 2007-07-04 at 09:07 +0000, walter harms wrote:
> you are right, normaly it would better to have two patches. IMHO for this special case
> (since removing a case is small change) you add the check.
Sure. Please find the updated patch below.
> The important point is to look not only for "the task" - like removing the cast - but also
> for other errors. i may take some time before the next will check that file again.
You're right. I initially thought that grouping changes logically makes
maintainer's job easy. But, in this case the change was too small.
---
Remove unnecessary cast of return value of kmalloc() and add kmalloc
checks in cris/arch-v32/mm/intmem.c
Signed-off-by: Suresh Jayaraman <sjayaraman@novell.com>
---
diff --git a/arch/cris/arch-v32/mm/intmem.c b/arch/cris/arch-v32/mm/intmem.c
index 41ee7f7..0f0b484 100644
--- a/arch/cris/arch-v32/mm/intmem.c
+++ b/arch/cris/arch-v32/mm/intmem.c
@@ -27,8 +27,12 @@ static void crisv32_intmem_init(void)
{
static int initiated = 0;
if (!initiated) {
- struct intmem_allocation* alloc - (struct intmem_allocation*)kmalloc(sizeof *alloc, GFP_KERNEL);
+ struct intmem_allocation *alloc = kmalloc(sizeof *alloc,
+ GFP_KERNEL);
+ if (!alloc) {
+ printk(KERN_ERR "crisv32: not enough memory\n");
+ return;
+ }
INIT_LIST_HEAD(&intmem_allocations);
intmem_virtual = ioremap(MEM_INTMEM_START, MEM_INTMEM_SIZE);
initiated = 1;
@@ -55,9 +59,13 @@ void* crisv32_intmem_alloc(unsigned size, unsigned align)
if (allocation->status = STATUS_FREE &&
allocation->size >= size + alignment) {
if (allocation->size > size + alignment) {
- struct intmem_allocation* alloc - (struct intmem_allocation*)
+ struct intmem_allocation *alloc kmalloc(sizeof *alloc, GFP_ATOMIC);
+ if (!alloc) {
+ printk(KERN_ERR "crisv32: not enough
+ memory\n");
+ return;
+ }
alloc->status = STATUS_FREE;
alloc->size = allocation->size - size - alignment;
alloc->offset = allocation->offset + size;
@@ -65,8 +73,12 @@ void* crisv32_intmem_alloc(unsigned size, unsigned align)
if (alignment) {
struct intmem_allocation* tmp;
- tmp = (struct intmem_allocation*)
- kmalloc(sizeof *tmp, GFP_ATOMIC);
+ tmp = kmalloc(sizeof *tmp, GFP_ATOMIC);
+ if (!tmp) {
+ printk(KERN_ERR "crisv32: not
+ enough memory\n");
+ return;
+ }
tmp->offset = allocation->offset;
tmp->size = alignment;
tmp->status = STATUS_FREE;
--
Suresh Jayaraman
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply related [flat|nested] 5+ messages in thread