lkml.org 
[lkml]   [2020]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] x86/mm: drop superfluous initialization


On Mon, 28 Sep 2020, Dave Hansen wrote:

> On 9/28/20 3:00 AM, Lukas Bulwahn wrote:
> > diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> > index c7a47603537f..5632f02146ca 100644
> > --- a/arch/x86/mm/init.c
> > +++ b/arch/x86/mm/init.c
> > @@ -609,7 +609,7 @@ static void __init memory_map_top_down(unsigned long map_start,
> > step_size = PMD_SIZE;
> > max_pfn_mapped = 0; /* will get exact value next */
> > min_pfn_mapped = real_end >> PAGE_SHIFT;
> > - last_start = start = real_end;
> > + last_start = real_end;
>
> Thanks for finding this.
>
> This becomes even more obviously correct if we just move the 'start'
> declaration into the while() loop. If we do that, it puts the three
> assignment locations right next to the definition, and its trivial to
> spot that the initialization was not missed:
>
> while (last_start > map_start) {
> unsigned long start;
>
> if (last_start > step_size) {
> start = round_down(last_start - 1, step_size);
> if (start < map_start)
> start = map_start;
> } else
> start = map_start;
> ...
>

Agree, this point is simply a question of style:

Shall local variables be defined as "local" as possible or simply
consistently at the beginning of each function?

If there are no strong opinions of style, I would just keep this patch
as-is.

> Either way, your patch looks correct to me:
>
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
>

Thanks for the Ack.

Lukas

\
 
 \ /
  Last update: 2020-09-29 10:42    [W:0.084 / U:4.000 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site