Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1470996

Re: [PATCH 1/5] IA64-IRQ: Use kmalloc_array() in sn_irq_lh_init()

Show all headers | View raw

On Fri, 2016-08-26 at 15:57 -0400, Julia Lawall wrote:
> On Fri, 26 Aug 2016, SF Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Fri, 26 Aug 2016 18:32:53 +0200
> >
> > * A multiplication for the size determination of a memory allocation
> >   indicated that an array data structure should be processed.
> >   Thus use the corresponding function "kmalloc_array".
> >
> >   This issue was detected by using the Coccinelle software.
> >
> > * Replace the specification of data structures by pointer dereferences
> >   to make the corresponding size determination a bit safer according to
> >   the Linux coding style convention.
> >
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > ---
> >  arch/ia64/sn/kernel/irq.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/ia64/sn/kernel/irq.c b/arch/ia64/sn/kernel/irq.c
[]
> > @@ -474,12 +474,12 @@ void __init sn_irq_lh_init(void)
> >  {
> >       int i;
> >
> > -     sn_irq_lh = kmalloc(sizeof(struct list_head *) * NR_IRQS, GFP_KERNEL);
> > +     sn_irq_lh = kmalloc_array(NR_IRQS, sizeof(*sn_irq_lh), GFP_KERNEL);
> >       if (!sn_irq_lh)
> >               panic("SN PCI INIT: Failed to allocate memory for PCI init\n");
> >
> >       for (i = 0; i < NR_IRQS; i++) {
> > -             sn_irq_lh[i] = kmalloc(sizeof(struct list_head), GFP_KERNEL);
> > +             sn_irq_lh[i] = kmalloc(*sn_irq_lh[i], GFP_KERNEL);
> 
> Did a sizeof get lost here?

Yes, thanks Julia.

This is why adding the generating spatch code is always good.

And Markus, please always compile test your code using the
appropriate cross-compilers available here:
https://www.kernel.org/pub/tools/crosstool/

And btw: using sizeof(*pp[i]) or sizeof(**pp) is not always
clearer or better than using sizeof(type)

If you _really wanted to clear up this code and make it more
robust/better, it'd probably be nicer to convert the
struct list_head **sn_irq_lh to a single struct list_head *
and do a single
	struct list_head *sn_irq_la = malloc_array(nr_irqs, sizeof(struct list_head);
instead of an malloc_array of the *sn_irq_lh and the
multiple individual struct list_head malloc entries
and change the INIT_LIST_HEAD and indexing code.

That would be less data space overall given the alignment
waste of the individual allocs.

It also appears that sn_irq_lh is extern in
arch/ia64/include/asm/sn/intr.h but could be made static
to arch/ia64/sn/kernel/irq.c.

But really, the conversion isn't worthwhile as NR_IRQS is
limited to much less than the maximum allocation / sizeof(ptr).

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 0/5] IA64: Fine-tuning for some function implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-26 20:10 +0200
  [PATCH 1/5] IA64-IRQ: Use kmalloc_array() in sn_irq_lh_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-26 20:10 +0200
    Re: [PATCH 1/5] IA64-IRQ: Use kmalloc_array() in sn_irq_lh_init() Julia Lawall <julia.lawall@lip6.fr> - 2016-08-26 22:00 +0200
      Re: [PATCH 1/5] IA64-IRQ: Use kmalloc_array() in sn_irq_lh_init() Joe Perches <joe@perches.com> - 2016-08-26 23:10 +0200
        Re: [PATCH 1/5] IA64-IRQ: Use kmalloc_array() in sn_irq_lh_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-27 09:10 +0200
          Re: [PATCH 1/5] IA64-IRQ: Use kmalloc_array() in sn_irq_lh_init() Joe Perches <joe@perches.com> - 2016-08-28 02:50 +0200
            Re: IA64-IRQ: Use kmalloc_array() in sn_irq_lh_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-28 09:40 +0200
            Re: [PATCH 1/5] IA64-IRQ: Use kmalloc_array() in sn_irq_lh_init() Julia Lawall <julia.lawall@lip6.fr> - 2016-08-28 11:30 +0200
              Re: [PATCH 1/5] IA64-IRQ: Use kmalloc_array() in sn_irq_lh_init() Joe Perches <joe@perches.com> - 2016-08-28 20:40 +0200
      Re: IA64-IRQ: Use kmalloc_array() in sn_irq_lh_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-27 08:30 +0200
    Re: [PATCH 1/5] IA64-IRQ: Use kmalloc_array() in sn_irq_lh_init() kbuild test robot <lkp@intel.com> - 2016-08-26 22:30 +0200
    Re: [PATCH 1/5] IA64-IRQ: Use kmalloc_array() in sn_irq_lh_init() walter harms <wharms@bfs.de> - 2016-08-27 10:50 +0200
  [PATCH 2/5] IA64-IRQ: Delete unnecessary braces SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-26 20:20 +0200
    Re: [PATCH 2/5] IA64-IRQ: Delete unnecessary braces kbuild test robot <lkp@intel.com> - 2016-08-26 22:30 +0200
  [PATCH 5/5] ia64/mm/tlb: Delete unnecessary braces SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-26 20:20 +0200
    Re: [PATCH 5/5] ia64/mm/tlb: Delete unnecessary braces kbuild test robot <lkp@intel.com> - 2016-08-26 22:50 +0200
      Re: ia64/mm/tlb: Delete unnecessary braces SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-27 09:30 +0200
  [PATCH 3/5] ia64/mm/tlb: Fix indentation in ia64_global_tlb_purge() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-26 20:20 +0200
  [PATCH 4/5] ia64/mm/tlb: Use kmalloc_array() in ia64_itr_entry() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-26 20:20 +0200

csiph-web