Re: [PATCH] arch/mips: remove duplicate structure field initialization

[Kevin D. Kissell]

Ha!  Found the breakage.  It wasn't a two-phase commit.  It was just an 
overworked mantainer in a hurry, in July 2007.  See commit 
033890b084adfa367c544864451d7730552ce8bf

Regards,

Kevin K.

Kevin D. Kissell wrote:
> I'm still on the mailing list, and had seen this going by.  I'm not 
> sure where that second .flags declaration got added.  Way, way back 
> when I was pretty much the only maintainer of the file, irq_ipi.flags 
> was explicitly  initialized to IRQF_DISABLED by an actual assignment 
> statement in setp_cross_vpe_interrupts(), and the per-CPUness was 
> handled by an "irq_desc[cpu_ipi_irq].status |= IRQ_PER_CPU".  My guess 
> is that first someone (maybe me) migrated the IRQF_DISABLED assignment 
> into the declaration of the struct, and that later someone found the 
> IRQ_PER_CPU thing bogus or deprecated and converted it into a second 
> .flags line in the struct declaration, missing the fact that there was 
> already one there.
>
> In any case, I'm willing to sign off on Julia's patch.  It's certainly 
> more important that the IRQ be PER_CPU than initially DISABLED, but 
> during the time when SMTC was seeing its heaviest testing at MIPS, 
> both attributes were true.
>
>          Regards,
>
>          Kevin K.
>
> Ralf Baechle wrote:
> > On Mon, Sep 21, 2009 at 05:08:55PM +0200, Julia Lawall wrote:
> >
> > Adding Kevin "SMTC Kissel to cc.
> >
> >  
> > > From: Julia Lawall <julia@xxxxxxx>
> > >
> > > The definition of the irq_ipi structure has two initializations of the
> > > flags field.  This combines them.
> > >
> > > The semantic match that finds this problem is as follows:
> > > (http://coccinelle.lip6.fr/)
> > >
> > > // <smpl>
> > > @r@
> > > identifier I, s, fld;
> > > position p0,p;
> > > expression E;
> > > @@
> > >
> > > struct I s =@p0 { ... .fld@p = E, ...};
> > >
> > > @s@
> > > identifier I, s, r.fld;
> > > position r.p0,p;
> > > expression E;
> > > @@
> > >
> > > struct I s =@p0 { ... .fld@p = E, ...};
> > >
> > > @script:python@
> > > p0 << r.p0;
> > > fld << r.fld;
> > > ps << s.p;
> > > pr << r.p;
> > > @@
> > >
> > > if int(ps[0].line)!=int(pr[0].line) or 
> > > int(ps[0].column)!=int(pr[0].column):
> > >   cocci.print_main(fld,p0)
> > > // </smpl>
> > >
> > > Signed-off-by: Julia Lawall <julia@xxxxxxx>
> > >
> > > ---
> > >  arch/mips/kernel/smtc.c             |    5 ++---
> > >  1 files changed, 2 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/mips/kernel/smtc.c b/arch/mips/kernel/smtc.c
> > > index 67153a0..4d181df 100644
> > > --- a/arch/mips/kernel/smtc.c
> > > +++ b/arch/mips/kernel/smtc.c
> > > @@ -1098,9 +1098,8 @@ static void ipi_irq_dispatch(void)
> > >  
> > >  static struct irqaction irq_ipi = {
> > >      .handler    = ipi_interrupt,
> > > -    .flags        = IRQF_DISABLED,
> > > -    .name        = "SMTC_IPI",
> > > -    .flags        = IRQF_PERCPU
> > > +    .flags        = IRQF_DISABLED | IRQF_PERCPU,
> > > +    .name        = "SMTC_IPI"
> > >  };
> > >  
> > >  static void setup_cross_vpe_interrupts(unsigned int nvpe)
> > >     
> >
> > The actual semantic of this code as implemented by gcc is that all 
> > but the
> > last initializer are ignored so until somebody actually tests your code
> > I'll just remove the first of the two initializers and put a comment 
> > there.
> >
> >   Ralf
> >   

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/