[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-arm] [PATCH 3/3] hw/misc/exynos4210_pmu: Reorder
From: |
Krzysztof Kozlowski |
Subject: |
Re: [Qemu-devel] [Qemu-arm] [PATCH 3/3] hw/misc/exynos4210_pmu: Reorder local variables for readability |
Date: |
Mon, 6 Mar 2017 19:23:46 +0200 |
User-agent: |
Mutt/1.6.2-neo (2016-08-21) |
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
- [Qemu-devel] [PATCH 2/3] hw/arm/exynos4210: Constify data pointed by few arguments and variables, (continued)