Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
From:   Lukas Bulwahn <lukas.bulwahn@gmail.com>
Date:   Thu, 3 Feb 2022 07:27:47 +0100
Message-ID: <CAKXUXMxFK9bo8jDoRZbQ0r2j-JwAGg3Xc5cpAcLaHfwHddJ7ew@mail.gmail.com>
Subject: Weird code with change "mm/gup: clean up follow_pfn_pte() slightly"
To:     Andrew Morton <akpm@linux-foundation.org>,
        John Hubbard <jhubbard@nvidia.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux-MM <linux-mm@kvack.org>
Cc:     Jason Gunthorpe <jgg@nvidia.com>, Peter Xu <peterx@redhat.com>,
        Alex Williamson <alex.williamson@redhat.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        David Hildenbrand <david@redhat.com>, Jan Kara <jack@suse.cz>,
        Jason Gunthorpe <jgg@ziepe.ca>,
        "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Dear John,

Your change "mm/gup: clean up follow_pfn_pte() slightly" (see Link),
visible in linux-next as commit 05fef840b5c6 ("mm/gup: clean up
follow_pfn_pte() slightly"), is somehow weird.

In the new branch if (pages), you set page = ERR_PTR(-EFAULT) and goto
out. However, at the label out, the value of page is not used, but the
return uses the variables i and ret.

Static analysis tools, such as clang-analyzer, rightfully complain
about such weird code.

Maybe you can have another look at what you intended to set in the
branch of that commit or if you intend to jump to the label out?


Best regards,

Lukas

Link: https://lkml.kernel.org/r/20220201101108.306062-3-jhubbard@nvidia.com