Subject: Re: [PATCH] mpic: make sparse happy

From: Milton Miller <[email-masked]>
Date: Wed, 20 Feb 2008 23:50:57 -0600

At Wed Feb 20 22:31:44 EST 2008, Johannes Berg wrote:
> I was running sparse on something else and noticed sparse warnings
> and especially the bogus code that is fixed by the first hunk of
> this patch, so I fixed them all while at it.

But your change is not equivalent!

> --- everything.orig/arch/powerpc/sysdev/mpic.c  2008-02-20 
> 12:25:41.000000000 +0100
> +++ everything/arch/powerpc/sysdev/mpic.c       2008-02-20 
> 12:28:37.000000000 +0100
> @@ -175,13 +175,13 @@ static inline void _mpic_write(enum mpic
>         switch(type) {
>  #ifdef CONFIG_PPC_DCR
>         case mpic_access_dcr:
> -               return dcr_write(rb->dhost, reg, value);
> +               dcr_write(rb->dhost, reg, value);
>  #endif
>         case mpic_access_mmio_be:
> -               return out_be32(rb->base + (reg >> 2), value);
> +               out_be32(rb->base + (reg >> 2), value);
>         case mpic_access_mmio_le:
>         default:
> -               return out_le32(rb->base + (reg >> 2), value);
> +               out_le32(rb->base + (reg >> 2), value);
>         }
>  }

You now write to the register with dcr, big, and little endian variants!

Either put a return or break after the calls to the void functions so 
you don't fall through.

...

> @@ -1107,10 +1108,10 @@ struct mpic * __init mpic_alloc(struct d
>          * in, try to obtain one
>          */
>         if (paddr == 0 && !(mpic->flags & MPIC_USES_DCR)) {
> -               const u32 *reg;
> -               reg = of_get_property(node, "reg", NULL);
> -               BUG_ON(reg == NULL);
> -               paddr = of_translate_address(node, reg);
> +               const u32 *regprop;
> +               regprop = of_get_property(node, "reg", NULL);
> +               BUG_ON(regprop == NULL);
> +               paddr = of_translate_address(node, regprop);
>                 BUG_ON(paddr == OF_BAD_ADDR);
>         }

This is reg variable is shadowed ... ok, although i might have renamed 
the outer one features or greg_feature.  For that matter, I would have 
initialized this reg/regprop on definition.