Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-26 Thread Jesper Juhl
On 26/08/07, Jan Engelhardt <[EMAIL PROTECTED]> wrote:
>
> On Aug 26 2007 15:28, Denys Vlasenko wrote:
> >> >
> >> > What exactely would using kcalloc() over kzalloc() here buy us?
> >>
> >> technically, nothing.
> >
> >The idea of calloc is that it can check for underflow in parameter.
>
> Actually, overflow.
>
> calloc(0x,  0x1000) => will return NULL
> malloc(0x * 0x1000) => silent 32 bit multiplication/truncation,
> will allocate less than requested.
>
> >calloc(-1, 1000) => easy to detect
> >malloc(-1 * 1000) => malloc(-1000) => not so trivial
>
Ok, that makes a bit of sense. Thank you.


-- 
Jesper Juhl <[EMAIL PROTECTED]>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please  http://www.expita.com/nomime.html
-
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/


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-26 Thread Jan Engelhardt

On Aug 26 2007 15:28, Denys Vlasenko wrote:
>> >
>> > What exactely would using kcalloc() over kzalloc() here buy us?
>>
>> technically, nothing.
>
>The idea of calloc is that it can check for underflow in parameter.

Actually, overflow.

calloc(0x,  0x1000) => will return NULL
malloc(0x * 0x1000) => silent 32 bit multiplication/truncation,
will allocate less than requested.

>calloc(-1, 1000) => easy to detect
>malloc(-1 * 1000) => malloc(-1000) => not so trivial


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


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-26 Thread Denys Vlasenko
On Sunday 26 August 2007 01:23, Robert P. J. Day wrote:
> On Sun, 26 Aug 2007, Jesper Juhl wrote:
> > On 26/08/07, Robert P. J. Day <[EMAIL PROTECTED]> wrote:
> > > i was thinking more along the lines of
> > >
> > > msp_parts[i] = kcalloc(pcnt, sizeof(struct mtd_partition), GFP_KERNEL);
> > >
> > > which was kind of the obvious implication, no?
> >
> > I guess
> >
> > > unless there's a reason kcalloc() wouldn't work here, this is
> > > pretty much what kcalloc() was designed for.
> >
> > When Denys brought up the zeroing thing and mentioned kzalloc() I
> > did consider kcalloc() instead, but kzalloc() makes this allocation
> > nicely look like the preceding ones visually and I couldn't convince
> > myself that kcalloc() would give us any real benefit here.
> >
> > What exactely would using kcalloc() over kzalloc() here buy us?
>
> technically, nothing.

The idea of calloc is that it can check for underflow in parameter.

calloc(-1, 1000) => easy to detect
malloc(-1 * 1000) => malloc(-1000) => not so trivial
--
vda
-
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/


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-26 Thread Denys Vlasenko
On Sunday 26 August 2007 01:23, Robert P. J. Day wrote:
 On Sun, 26 Aug 2007, Jesper Juhl wrote:
  On 26/08/07, Robert P. J. Day [EMAIL PROTECTED] wrote:
   i was thinking more along the lines of
  
   msp_parts[i] = kcalloc(pcnt, sizeof(struct mtd_partition), GFP_KERNEL);
  
   which was kind of the obvious implication, no?
 
  I guess
 
   unless there's a reason kcalloc() wouldn't work here, this is
   pretty much what kcalloc() was designed for.
 
  When Denys brought up the zeroing thing and mentioned kzalloc() I
  did consider kcalloc() instead, but kzalloc() makes this allocation
  nicely look like the preceding ones visually and I couldn't convince
  myself that kcalloc() would give us any real benefit here.
 
  What exactely would using kcalloc() over kzalloc() here buy us?

 technically, nothing.

The idea of calloc is that it can check for underflow in parameter.

calloc(-1, 1000) = easy to detect
malloc(-1 * 1000) = malloc(-1000) = not so trivial
--
vda
-
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/


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-26 Thread Jan Engelhardt

On Aug 26 2007 15:28, Denys Vlasenko wrote:
 
  What exactely would using kcalloc() over kzalloc() here buy us?

 technically, nothing.

The idea of calloc is that it can check for underflow in parameter.

Actually, overflow.

calloc(0x,  0x1000) = will return NULL
malloc(0x * 0x1000) = silent 32 bit multiplication/truncation,
will allocate less than requested.

calloc(-1, 1000) = easy to detect
malloc(-1 * 1000) = malloc(-1000) = not so trivial


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


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-26 Thread Jesper Juhl
On 26/08/07, Jan Engelhardt [EMAIL PROTECTED] wrote:

 On Aug 26 2007 15:28, Denys Vlasenko wrote:
  
   What exactely would using kcalloc() over kzalloc() here buy us?
 
  technically, nothing.
 
 The idea of calloc is that it can check for underflow in parameter.

 Actually, overflow.

 calloc(0x,  0x1000) = will return NULL
 malloc(0x * 0x1000) = silent 32 bit multiplication/truncation,
 will allocate less than requested.

 calloc(-1, 1000) = easy to detect
 malloc(-1 * 1000) = malloc(-1000) = not so trivial

Ok, that makes a bit of sense. Thank you.


-- 
Jesper Juhl [EMAIL PROTECTED]
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please  http://www.expita.com/nomime.html
-
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/


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-25 Thread Kyle Moffett

On Aug 25, 2007, at 20:36:32, Jesper Juhl wrote:

On 26/08/07, Robert P. J. Day <[EMAIL PROTECTED]> wrote:
technically, nothing.  but if you're not going to use kcalloc()  
when you're explicitly allocating an array of identical objects  
(that you want zero-filled, as a bonus), then what's the point of  
ever having defined a kcalloc() routine in the first place?



I wonder a bit about that myself...

I have found some other issues in that function that I want to fix,  
so I'll be respinning the patch as a patch series instead - and why  
not; I'll just go with kcalloc() and see what the maintainers have  
to say, it's not like I personally care much one way or the other.


I think the original reasoning behind kcalloc() was that it did some  
extra input checking, so that if the product of the two numbers  
overflowed, it would fail with NULL instead of allocating  
insufficient space.  In the kernel it doesn't matter in practice  
since you MUST have additional checking on the size of allocated  
memory anyways, not even considering the fact that >PAGE_SIZE  
allocations are probably going to fail with decent frequency regardless.


Cheers,
Kyle Moffett


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


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-25 Thread Jesper Juhl
On 26/08/07, Robert P. J. Day <[EMAIL PROTECTED]> wrote:
> On Sun, 26 Aug 2007, Jesper Juhl wrote:
>
> > On 26/08/07, Robert P. J. Day <[EMAIL PROTECTED]> wrote:
>
> > > i was thinking more along the lines of
> > >
> > > msp_parts[i] = kcalloc(pcnt, sizeof(struct mtd_partition), GFP_KERNEL);
> > >
> > > which was kind of the obvious implication, no?
> >
> > I guess
> >
> > > unless there's a reason kcalloc() wouldn't work here, this is
> > > pretty much what kcalloc() was designed for.
> > >
> > When Denys brought up the zeroing thing and mentioned kzalloc() I
> > did consider kcalloc() instead, but kzalloc() makes this allocation
> > nicely look like the preceding ones visually and I couldn't convince
> > myself that kcalloc() would give us any real benefit here.
> >
> > What exactely would using kcalloc() over kzalloc() here buy us?
>
> technically, nothing.  but if you're not going to use kcalloc() when
> you're explicitly allocating an array of identical objects (that you
> want zero-filled, as a bonus), then what's the point of ever having
> defined a kcalloc() routine in the first place?
>
I wonder a bit about that myself...

I have found some other issues in that function that I want to fix, so
I'll be respinning the patch as a patch series instead - and why not;
I'll just go with kcalloc() and see what the maintainers have to say,
it's not like I personally care much one way or the other.

-- 
Jesper Juhl <[EMAIL PROTECTED]>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please  http://www.expita.com/nomime.html
-
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/


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-25 Thread Robert P. J. Day
On Sun, 26 Aug 2007, Jesper Juhl wrote:

> On 26/08/07, Robert P. J. Day <[EMAIL PROTECTED]> wrote:

> > i was thinking more along the lines of
> >
> > msp_parts[i] = kcalloc(pcnt, sizeof(struct mtd_partition), GFP_KERNEL);
> >
> > which was kind of the obvious implication, no?
>
> I guess
>
> > unless there's a reason kcalloc() wouldn't work here, this is
> > pretty much what kcalloc() was designed for.
> >
> When Denys brought up the zeroing thing and mentioned kzalloc() I
> did consider kcalloc() instead, but kzalloc() makes this allocation
> nicely look like the preceding ones visually and I couldn't convince
> myself that kcalloc() would give us any real benefit here.
>
> What exactely would using kcalloc() over kzalloc() here buy us?

technically, nothing.  but if you're not going to use kcalloc() when
you're explicitly allocating an array of identical objects (that you
want zero-filled, as a bonus), then what's the point of ever having
defined a kcalloc() routine in the first place?

rday
-- 

Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca

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


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-25 Thread Jesper Juhl
On 26/08/07, Robert P. J. Day <[EMAIL PROTECTED]> wrote:
> On Sun, 26 Aug 2007, Jesper Juhl wrote:
>
> > On 24/08/07, Robert P. J. Day <[EMAIL PROTECTED]> wrote:
>
> > > actually, i would think kcalloc would be more appropriate here, no?
> > >
> >
> > Why?
> >
> > msp_parts[i] = kzalloc(pcnt * sizeof(struct mtd_partition), GFP_KERNEL);
> >
> > seems better to me than
> >
> > msp_parts[i] = kcalloc(1, pcnt * sizeof(struct mtd_partition), GFP_KERNEL);
>
> i was thinking more along the lines of
>
> msp_parts[i] = kcalloc(pcnt, sizeof(struct mtd_partition), GFP_KERNEL);
>
> which was kind of the obvious implication, no?

I guess

> unless there's a
> reason kcalloc() wouldn't work here, this is pretty much what
> kcalloc() was designed for.
>
When Denys brought up the zeroing thing and mentioned kzalloc() I did
consider kcalloc() instead, but kzalloc() makes this allocation nicely
look like the preceding ones visually and I couldn't convince myself
that kcalloc() would give us any real benefit here.

What exactely would using kcalloc() over kzalloc() here buy us?

-- 
Jesper Juhl <[EMAIL PROTECTED]>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please  http://www.expita.com/nomime.html
-
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/


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-25 Thread Robert P. J. Day
On Sun, 26 Aug 2007, Jesper Juhl wrote:

> On 24/08/07, Robert P. J. Day <[EMAIL PROTECTED]> wrote:

> > actually, i would think kcalloc would be more appropriate here, no?
> >
>
> Why?
>
> msp_parts[i] = kzalloc(pcnt * sizeof(struct mtd_partition), GFP_KERNEL);
>
> seems better to me than
>
> msp_parts[i] = kcalloc(1, pcnt * sizeof(struct mtd_partition), GFP_KERNEL);

i was thinking more along the lines of

msp_parts[i] = kcalloc(pcnt, sizeof(struct mtd_partition), GFP_KERNEL);

which was kind of the obvious implication, no?  unless there's a
reason kcalloc() wouldn't work here, this is pretty much what
kcalloc() was designed for.

rday

-- 

Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca

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


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-25 Thread Jesper Juhl
On 24/08/07, Robert P. J. Day <[EMAIL PROTECTED]> wrote:
> On Fri, 24 Aug 2007, Denys Vlasenko wrote:
>
> > On Friday 24 August 2007 00:52, Jesper Juhl wrote:
> > > kmalloc() returns a void pointer.
> > > No need to cast it.
> >
> > > -   msp_flash = (struct mtd_info **)kmalloc(
> > > -   fcnt * sizeof(struct map_info *), GFP_KERNEL);
> > > -   msp_parts = (struct mtd_partition **)kmalloc(
> > > -   fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
> > > -   msp_maps = (struct map_info *)kmalloc(
> > > -   fcnt * sizeof(struct mtd_info), GFP_KERNEL);
> > > +   msp_flash = kmalloc(fcnt * sizeof(struct map_info *), GFP_KERNEL);
> > > +   msp_parts = kmalloc(fcnt * sizeof(struct mtd_partition *), 
> > > GFP_KERNEL);
> > > +   msp_maps = kmalloc(fcnt * sizeof(struct mtd_info), GFP_KERNEL);
> > > memset(msp_maps, 0, fcnt * sizeof(struct mtd_info));
> >
> > This one wants kzalloc.
> >
> > > -   msp_parts[i] = (struct mtd_partition *)kmalloc(
> > > -   pcnt * sizeof(struct mtd_partition), GFP_KERNEL);
> > > +   msp_parts[i] = kmalloc(pcnt * sizeof(struct mtd_partition),
> > > +   GFP_KERNEL);
> > > memset(msp_parts[i], 0, pcnt * sizeof(struct mtd_partition));
> > >
> > > /* now initialize the devices proper */
> >
> > Same
>
> actually, i would think kcalloc would be more appropriate here, no?
>

Why?

msp_parts[i] = kzalloc(pcnt * sizeof(struct mtd_partition), GFP_KERNEL);

seems better to me than

msp_parts[i] = kcalloc(1, pcnt * sizeof(struct mtd_partition), GFP_KERNEL);


-- 
Jesper Juhl <[EMAIL PROTECTED]>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please  http://www.expita.com/nomime.html
-
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/


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-25 Thread Jesper Juhl
On 24/08/07, Robert P. J. Day [EMAIL PROTECTED] wrote:
 On Fri, 24 Aug 2007, Denys Vlasenko wrote:

  On Friday 24 August 2007 00:52, Jesper Juhl wrote:
   kmalloc() returns a void pointer.
   No need to cast it.
 
   -   msp_flash = (struct mtd_info **)kmalloc(
   -   fcnt * sizeof(struct map_info *), GFP_KERNEL);
   -   msp_parts = (struct mtd_partition **)kmalloc(
   -   fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
   -   msp_maps = (struct map_info *)kmalloc(
   -   fcnt * sizeof(struct mtd_info), GFP_KERNEL);
   +   msp_flash = kmalloc(fcnt * sizeof(struct map_info *), GFP_KERNEL);
   +   msp_parts = kmalloc(fcnt * sizeof(struct mtd_partition *), 
   GFP_KERNEL);
   +   msp_maps = kmalloc(fcnt * sizeof(struct mtd_info), GFP_KERNEL);
   memset(msp_maps, 0, fcnt * sizeof(struct mtd_info));
 
  This one wants kzalloc.
 
   -   msp_parts[i] = (struct mtd_partition *)kmalloc(
   -   pcnt * sizeof(struct mtd_partition), GFP_KERNEL);
   +   msp_parts[i] = kmalloc(pcnt * sizeof(struct mtd_partition),
   +   GFP_KERNEL);
   memset(msp_parts[i], 0, pcnt * sizeof(struct mtd_partition));
  
   /* now initialize the devices proper */
 
  Same

 actually, i would think kcalloc would be more appropriate here, no?


Why?

msp_parts[i] = kzalloc(pcnt * sizeof(struct mtd_partition), GFP_KERNEL);

seems better to me than

msp_parts[i] = kcalloc(1, pcnt * sizeof(struct mtd_partition), GFP_KERNEL);


-- 
Jesper Juhl [EMAIL PROTECTED]
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please  http://www.expita.com/nomime.html
-
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/


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-25 Thread Robert P. J. Day
On Sun, 26 Aug 2007, Jesper Juhl wrote:

 On 24/08/07, Robert P. J. Day [EMAIL PROTECTED] wrote:

  actually, i would think kcalloc would be more appropriate here, no?
 

 Why?

 msp_parts[i] = kzalloc(pcnt * sizeof(struct mtd_partition), GFP_KERNEL);

 seems better to me than

 msp_parts[i] = kcalloc(1, pcnt * sizeof(struct mtd_partition), GFP_KERNEL);

i was thinking more along the lines of

msp_parts[i] = kcalloc(pcnt, sizeof(struct mtd_partition), GFP_KERNEL);

which was kind of the obvious implication, no?  unless there's a
reason kcalloc() wouldn't work here, this is pretty much what
kcalloc() was designed for.

rday

-- 

Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca

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


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-25 Thread Jesper Juhl
On 26/08/07, Robert P. J. Day [EMAIL PROTECTED] wrote:
 On Sun, 26 Aug 2007, Jesper Juhl wrote:

  On 24/08/07, Robert P. J. Day [EMAIL PROTECTED] wrote:

   actually, i would think kcalloc would be more appropriate here, no?
  
 
  Why?
 
  msp_parts[i] = kzalloc(pcnt * sizeof(struct mtd_partition), GFP_KERNEL);
 
  seems better to me than
 
  msp_parts[i] = kcalloc(1, pcnt * sizeof(struct mtd_partition), GFP_KERNEL);

 i was thinking more along the lines of

 msp_parts[i] = kcalloc(pcnt, sizeof(struct mtd_partition), GFP_KERNEL);

 which was kind of the obvious implication, no?

I guess

 unless there's a
 reason kcalloc() wouldn't work here, this is pretty much what
 kcalloc() was designed for.

When Denys brought up the zeroing thing and mentioned kzalloc() I did
consider kcalloc() instead, but kzalloc() makes this allocation nicely
look like the preceding ones visually and I couldn't convince myself
that kcalloc() would give us any real benefit here.

What exactely would using kcalloc() over kzalloc() here buy us?

-- 
Jesper Juhl [EMAIL PROTECTED]
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please  http://www.expita.com/nomime.html
-
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/


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-25 Thread Robert P. J. Day
On Sun, 26 Aug 2007, Jesper Juhl wrote:

 On 26/08/07, Robert P. J. Day [EMAIL PROTECTED] wrote:

  i was thinking more along the lines of
 
  msp_parts[i] = kcalloc(pcnt, sizeof(struct mtd_partition), GFP_KERNEL);
 
  which was kind of the obvious implication, no?

 I guess

  unless there's a reason kcalloc() wouldn't work here, this is
  pretty much what kcalloc() was designed for.
 
 When Denys brought up the zeroing thing and mentioned kzalloc() I
 did consider kcalloc() instead, but kzalloc() makes this allocation
 nicely look like the preceding ones visually and I couldn't convince
 myself that kcalloc() would give us any real benefit here.

 What exactely would using kcalloc() over kzalloc() here buy us?

technically, nothing.  but if you're not going to use kcalloc() when
you're explicitly allocating an array of identical objects (that you
want zero-filled, as a bonus), then what's the point of ever having
defined a kcalloc() routine in the first place?

rday
-- 

Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca

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


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-25 Thread Jesper Juhl
On 26/08/07, Robert P. J. Day [EMAIL PROTECTED] wrote:
 On Sun, 26 Aug 2007, Jesper Juhl wrote:

  On 26/08/07, Robert P. J. Day [EMAIL PROTECTED] wrote:

   i was thinking more along the lines of
  
   msp_parts[i] = kcalloc(pcnt, sizeof(struct mtd_partition), GFP_KERNEL);
  
   which was kind of the obvious implication, no?
 
  I guess
 
   unless there's a reason kcalloc() wouldn't work here, this is
   pretty much what kcalloc() was designed for.
  
  When Denys brought up the zeroing thing and mentioned kzalloc() I
  did consider kcalloc() instead, but kzalloc() makes this allocation
  nicely look like the preceding ones visually and I couldn't convince
  myself that kcalloc() would give us any real benefit here.
 
  What exactely would using kcalloc() over kzalloc() here buy us?

 technically, nothing.  but if you're not going to use kcalloc() when
 you're explicitly allocating an array of identical objects (that you
 want zero-filled, as a bonus), then what's the point of ever having
 defined a kcalloc() routine in the first place?

I wonder a bit about that myself...

I have found some other issues in that function that I want to fix, so
I'll be respinning the patch as a patch series instead - and why not;
I'll just go with kcalloc() and see what the maintainers have to say,
it's not like I personally care much one way or the other.

-- 
Jesper Juhl [EMAIL PROTECTED]
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please  http://www.expita.com/nomime.html
-
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/


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-25 Thread Kyle Moffett

On Aug 25, 2007, at 20:36:32, Jesper Juhl wrote:

On 26/08/07, Robert P. J. Day [EMAIL PROTECTED] wrote:
technically, nothing.  but if you're not going to use kcalloc()  
when you're explicitly allocating an array of identical objects  
(that you want zero-filled, as a bonus), then what's the point of  
ever having defined a kcalloc() routine in the first place?



I wonder a bit about that myself...

I have found some other issues in that function that I want to fix,  
so I'll be respinning the patch as a patch series instead - and why  
not; I'll just go with kcalloc() and see what the maintainers have  
to say, it's not like I personally care much one way or the other.


I think the original reasoning behind kcalloc() was that it did some  
extra input checking, so that if the product of the two numbers  
overflowed, it would fail with NULL instead of allocating  
insufficient space.  In the kernel it doesn't matter in practice  
since you MUST have additional checking on the size of allocated  
memory anyways, not even considering the fact that PAGE_SIZE  
allocations are probably going to fail with decent frequency regardless.


Cheers,
Kyle Moffett


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


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-24 Thread Robert P. J. Day
On Fri, 24 Aug 2007, Denys Vlasenko wrote:

> On Friday 24 August 2007 00:52, Jesper Juhl wrote:
> > kmalloc() returns a void pointer.
> > No need to cast it.
>
> > -   msp_flash = (struct mtd_info **)kmalloc(
> > -   fcnt * sizeof(struct map_info *), GFP_KERNEL);
> > -   msp_parts = (struct mtd_partition **)kmalloc(
> > -   fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
> > -   msp_maps = (struct map_info *)kmalloc(
> > -   fcnt * sizeof(struct mtd_info), GFP_KERNEL);
> > +   msp_flash = kmalloc(fcnt * sizeof(struct map_info *), GFP_KERNEL);
> > +   msp_parts = kmalloc(fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
> > +   msp_maps = kmalloc(fcnt * sizeof(struct mtd_info), GFP_KERNEL);
> > memset(msp_maps, 0, fcnt * sizeof(struct mtd_info));
>
> This one wants kzalloc.
>
> > -   msp_parts[i] = (struct mtd_partition *)kmalloc(
> > -   pcnt * sizeof(struct mtd_partition), GFP_KERNEL);
> > +   msp_parts[i] = kmalloc(pcnt * sizeof(struct mtd_partition),
> > +   GFP_KERNEL);
> > memset(msp_parts[i], 0, pcnt * sizeof(struct mtd_partition));
> >
> > /* now initialize the devices proper */
>
> Same

actually, i would think kcalloc would be more appropriate here, no?

rday
-- 

Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca

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


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-24 Thread Jesper Juhl
On 24/08/07, Denys Vlasenko <[EMAIL PROTECTED]> wrote:
> On Friday 24 August 2007 00:52, Jesper Juhl wrote:
> > kmalloc() returns a void pointer.
> > No need to cast it.
>
> > - msp_flash = (struct mtd_info **)kmalloc(
> > - fcnt * sizeof(struct map_info *), GFP_KERNEL);
> > - msp_parts = (struct mtd_partition **)kmalloc(
> > - fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
> > - msp_maps = (struct map_info *)kmalloc(
> > - fcnt * sizeof(struct mtd_info), GFP_KERNEL);
> > + msp_flash = kmalloc(fcnt * sizeof(struct map_info *), GFP_KERNEL);
> > + msp_parts = kmalloc(fcnt * sizeof(struct mtd_partition *), 
> > GFP_KERNEL);
> > + msp_maps = kmalloc(fcnt * sizeof(struct mtd_info), GFP_KERNEL);
> >   memset(msp_maps, 0, fcnt * sizeof(struct mtd_info));
>
> This one wants kzalloc.
>
> > - msp_parts[i] = (struct mtd_partition *)kmalloc(
> > - pcnt * sizeof(struct mtd_partition), GFP_KERNEL);
> > + msp_parts[i] = kmalloc(pcnt * sizeof(struct mtd_partition),
> > + GFP_KERNEL);
> >   memset(msp_parts[i], 0, pcnt * sizeof(struct mtd_partition));
> >
> >   /* now initialize the devices proper */
>
> Same

Ok, thank you for that feedback.
 I'll respin the patch with that change when I resubmit all the ones
that don't get picked up (probably next week).

-- 
Jesper Juhl <[EMAIL PROTECTED]>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please  http://www.expita.com/nomime.html
-
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/


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-24 Thread Denys Vlasenko
On Friday 24 August 2007 00:52, Jesper Juhl wrote:
> kmalloc() returns a void pointer.
> No need to cast it.

> - msp_flash = (struct mtd_info **)kmalloc(
> - fcnt * sizeof(struct map_info *), GFP_KERNEL);
> - msp_parts = (struct mtd_partition **)kmalloc(
> - fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
> - msp_maps = (struct map_info *)kmalloc(
> - fcnt * sizeof(struct mtd_info), GFP_KERNEL);
> + msp_flash = kmalloc(fcnt * sizeof(struct map_info *), GFP_KERNEL);
> + msp_parts = kmalloc(fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
> + msp_maps = kmalloc(fcnt * sizeof(struct mtd_info), GFP_KERNEL);
>   memset(msp_maps, 0, fcnt * sizeof(struct mtd_info));

This one wants kzalloc.

> - msp_parts[i] = (struct mtd_partition *)kmalloc(
> - pcnt * sizeof(struct mtd_partition), GFP_KERNEL);
> + msp_parts[i] = kmalloc(pcnt * sizeof(struct mtd_partition),
> + GFP_KERNEL);
>   memset(msp_parts[i], 0, pcnt * sizeof(struct mtd_partition));
>
>   /* now initialize the devices proper */

Same
--
vda
-
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/


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-24 Thread Denys Vlasenko
On Friday 24 August 2007 00:52, Jesper Juhl wrote:
 kmalloc() returns a void pointer.
 No need to cast it.

 - msp_flash = (struct mtd_info **)kmalloc(
 - fcnt * sizeof(struct map_info *), GFP_KERNEL);
 - msp_parts = (struct mtd_partition **)kmalloc(
 - fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
 - msp_maps = (struct map_info *)kmalloc(
 - fcnt * sizeof(struct mtd_info), GFP_KERNEL);
 + msp_flash = kmalloc(fcnt * sizeof(struct map_info *), GFP_KERNEL);
 + msp_parts = kmalloc(fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
 + msp_maps = kmalloc(fcnt * sizeof(struct mtd_info), GFP_KERNEL);
   memset(msp_maps, 0, fcnt * sizeof(struct mtd_info));

This one wants kzalloc.

 - msp_parts[i] = (struct mtd_partition *)kmalloc(
 - pcnt * sizeof(struct mtd_partition), GFP_KERNEL);
 + msp_parts[i] = kmalloc(pcnt * sizeof(struct mtd_partition),
 + GFP_KERNEL);
   memset(msp_parts[i], 0, pcnt * sizeof(struct mtd_partition));

   /* now initialize the devices proper */

Same
--
vda
-
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/


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-24 Thread Jesper Juhl
On 24/08/07, Denys Vlasenko [EMAIL PROTECTED] wrote:
 On Friday 24 August 2007 00:52, Jesper Juhl wrote:
  kmalloc() returns a void pointer.
  No need to cast it.

  - msp_flash = (struct mtd_info **)kmalloc(
  - fcnt * sizeof(struct map_info *), GFP_KERNEL);
  - msp_parts = (struct mtd_partition **)kmalloc(
  - fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
  - msp_maps = (struct map_info *)kmalloc(
  - fcnt * sizeof(struct mtd_info), GFP_KERNEL);
  + msp_flash = kmalloc(fcnt * sizeof(struct map_info *), GFP_KERNEL);
  + msp_parts = kmalloc(fcnt * sizeof(struct mtd_partition *), 
  GFP_KERNEL);
  + msp_maps = kmalloc(fcnt * sizeof(struct mtd_info), GFP_KERNEL);
memset(msp_maps, 0, fcnt * sizeof(struct mtd_info));

 This one wants kzalloc.

  - msp_parts[i] = (struct mtd_partition *)kmalloc(
  - pcnt * sizeof(struct mtd_partition), GFP_KERNEL);
  + msp_parts[i] = kmalloc(pcnt * sizeof(struct mtd_partition),
  + GFP_KERNEL);
memset(msp_parts[i], 0, pcnt * sizeof(struct mtd_partition));
 
/* now initialize the devices proper */

 Same

Ok, thank you for that feedback.
 I'll respin the patch with that change when I resubmit all the ones
that don't get picked up (probably next week).

-- 
Jesper Juhl [EMAIL PROTECTED]
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please  http://www.expita.com/nomime.html
-
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/


Re: [PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-24 Thread Robert P. J. Day
On Fri, 24 Aug 2007, Denys Vlasenko wrote:

 On Friday 24 August 2007 00:52, Jesper Juhl wrote:
  kmalloc() returns a void pointer.
  No need to cast it.

  -   msp_flash = (struct mtd_info **)kmalloc(
  -   fcnt * sizeof(struct map_info *), GFP_KERNEL);
  -   msp_parts = (struct mtd_partition **)kmalloc(
  -   fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
  -   msp_maps = (struct map_info *)kmalloc(
  -   fcnt * sizeof(struct mtd_info), GFP_KERNEL);
  +   msp_flash = kmalloc(fcnt * sizeof(struct map_info *), GFP_KERNEL);
  +   msp_parts = kmalloc(fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
  +   msp_maps = kmalloc(fcnt * sizeof(struct mtd_info), GFP_KERNEL);
  memset(msp_maps, 0, fcnt * sizeof(struct mtd_info));

 This one wants kzalloc.

  -   msp_parts[i] = (struct mtd_partition *)kmalloc(
  -   pcnt * sizeof(struct mtd_partition), GFP_KERNEL);
  +   msp_parts[i] = kmalloc(pcnt * sizeof(struct mtd_partition),
  +   GFP_KERNEL);
  memset(msp_parts[i], 0, pcnt * sizeof(struct mtd_partition));
 
  /* now initialize the devices proper */

 Same

actually, i would think kcalloc would be more appropriate here, no?

rday
-- 

Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca

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


[PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-23 Thread Jesper Juhl
kmalloc() returns a void pointer.
No need to cast it.

Signed-off-by: Jesper Juhl <[EMAIL PROTECTED]>
---
 drivers/mtd/maps/pmcmsp-flash.c |   13 +
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/maps/pmcmsp-flash.c b/drivers/mtd/maps/pmcmsp-flash.c
index 7e0377e..dfdb120 100644
--- a/drivers/mtd/maps/pmcmsp-flash.c
+++ b/drivers/mtd/maps/pmcmsp-flash.c
@@ -73,12 +73,9 @@ int __init init_msp_flash(void)
return -ENXIO;
 
printk(KERN_NOTICE "Found %d PMC flash devices\n", fcnt);
-   msp_flash = (struct mtd_info **)kmalloc(
-   fcnt * sizeof(struct map_info *), GFP_KERNEL);
-   msp_parts = (struct mtd_partition **)kmalloc(
-   fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
-   msp_maps = (struct map_info *)kmalloc(
-   fcnt * sizeof(struct mtd_info), GFP_KERNEL);
+   msp_flash = kmalloc(fcnt * sizeof(struct map_info *), GFP_KERNEL);
+   msp_parts = kmalloc(fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
+   msp_maps = kmalloc(fcnt * sizeof(struct mtd_info), GFP_KERNEL);
memset(msp_maps, 0, fcnt * sizeof(struct mtd_info));
 
/* loop over the flash devices, initializing each */
@@ -95,8 +92,8 @@ int __init init_msp_flash(void)
continue;
}
 
-   msp_parts[i] = (struct mtd_partition *)kmalloc(
-   pcnt * sizeof(struct mtd_partition), GFP_KERNEL);
+   msp_parts[i] = kmalloc(pcnt * sizeof(struct mtd_partition),
+   GFP_KERNEL);
memset(msp_parts[i], 0, pcnt * sizeof(struct mtd_partition));
 
/* now initialize the devices proper */
-- 
1.5.2.2

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


[PATCH 09/30] mtd: Don't cast kmalloc() return value in drivers/mtd/maps/pmcmsp-flash.c

2007-08-23 Thread Jesper Juhl
kmalloc() returns a void pointer.
No need to cast it.

Signed-off-by: Jesper Juhl [EMAIL PROTECTED]
---
 drivers/mtd/maps/pmcmsp-flash.c |   13 +
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/maps/pmcmsp-flash.c b/drivers/mtd/maps/pmcmsp-flash.c
index 7e0377e..dfdb120 100644
--- a/drivers/mtd/maps/pmcmsp-flash.c
+++ b/drivers/mtd/maps/pmcmsp-flash.c
@@ -73,12 +73,9 @@ int __init init_msp_flash(void)
return -ENXIO;
 
printk(KERN_NOTICE Found %d PMC flash devices\n, fcnt);
-   msp_flash = (struct mtd_info **)kmalloc(
-   fcnt * sizeof(struct map_info *), GFP_KERNEL);
-   msp_parts = (struct mtd_partition **)kmalloc(
-   fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
-   msp_maps = (struct map_info *)kmalloc(
-   fcnt * sizeof(struct mtd_info), GFP_KERNEL);
+   msp_flash = kmalloc(fcnt * sizeof(struct map_info *), GFP_KERNEL);
+   msp_parts = kmalloc(fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
+   msp_maps = kmalloc(fcnt * sizeof(struct mtd_info), GFP_KERNEL);
memset(msp_maps, 0, fcnt * sizeof(struct mtd_info));
 
/* loop over the flash devices, initializing each */
@@ -95,8 +92,8 @@ int __init init_msp_flash(void)
continue;
}
 
-   msp_parts[i] = (struct mtd_partition *)kmalloc(
-   pcnt * sizeof(struct mtd_partition), GFP_KERNEL);
+   msp_parts[i] = kmalloc(pcnt * sizeof(struct mtd_partition),
+   GFP_KERNEL);
memset(msp_parts[i], 0, pcnt * sizeof(struct mtd_partition));
 
/* now initialize the devices proper */
-- 
1.5.2.2

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