Re: [patch 176/178] mm/page_alloc: redundant definition variables of pfn in for loop

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Apr 29, 2021 at 11:02 PM Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> This variable pfn is defined repeatedly, so it can be deleted.

I'd actually much prefer this patch to be done exactly the other way:
avoid the variable name shadowing not by removing the second
declaration, but by actually making *both* declarations local to the
loops.

The lifetime of those 'pfn' variables really is just the inner body of
the loop, not the whole function.

Now, a good compiler will notice that the uses are entirely disjoint,
and not care about how the programmer used the same variable for two
different cases, but it's good practice to just minimize the scope of
a variable.

So I'm dropping this, but I would apply a patch that did something
like this instead

   void free_unref_page_list(struct list_head *list)
   {
        struct page *page, *next;
  -     unsigned long flags, pfn;
  +     unsigned long flags;
        int batch_count = 0;

        /* Prepare pages for freeing */
        list_for_each_entry_safe(page, next, list, lru) {
  -             pfn = page_to_pfn(page);
  +             unsigned long pfn = page_to_pfn(page);
                if (!free_unref_page_prepare(page, pfn))
                        list_del(&page->lru);
                set_page_private(page, pfn);

(UNTESTED, and intentionally whitespace-damaged, you get the idea).

        Linus



[Index of Archives]     [Kernel Archive]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]

  Powered by Linux