Re: [Qemu-devel] [Qemu-arm] [PATCH 3/3] hw/misc/exynos4210_pmu: Reorder local variables for readability

[Krzysztof Kozlowski]

On Mon, Mar 06, 2017 at 01:06:29AM -0300, Philippe Mathieu-Daudé wrote:
> Hi Krzysztof,
> 
> On 03/05/2017 06:48 PM, Krzysztof Kozlowski wrote:
> > Short declaration of 'i' was in the middle of declarations with
> > assignments.  Make it a little bit more readable.  No functional change.
> > 
> > Signed-off-by: Krzysztof Kozlowski <address@hidden>
> > ---
> >  hw/misc/exynos4210_pmu.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/misc/exynos4210_pmu.c b/hw/misc/exynos4210_pmu.c
> > index cbdfa0614600..60d1545c0baa 100644
> > --- a/hw/misc/exynos4210_pmu.c
> > +++ b/hw/misc/exynos4210_pmu.c
> > @@ -401,8 +401,8 @@ static uint64_t exynos4210_pmu_read(void *opaque, 
> > hwaddr offset,
> >                                      unsigned size)
> >  {
> >      const Exynos4210PmuState *s = (Exynos4210PmuState *)opaque;
> > -    unsigned i;
> >      const Exynos4210PmuReg *reg_p = exynos4210_pmu_regs;
> > +    unsigned i;
> 
> your change seems OK but while you are here, 'unsigned' is considered
> harmful since more than a decade. why not use 'size_t i' since
> PMU_NUM_OF_REGISTERS is indeed an ARRAY_SIZE()?

It is a loop counter. I think "unsigned int" for loop counters is the
most common pattern - easy to understand the true meaning of variable.
I can expand it to "unsigned int" but that wouldn't change much...

Let's look at current style:
$ git grep "unsigned int i[,;$]" | wc -l
188

$ git grep "unsigned i[,;$]" | wc -l
92

$ git grep "size_t i[,;$]" | wc -l
114

... where do we go now?



Best regards,
Krzysztof