From: Sean Christopherson <seanjc@google.com>
To: Ackerley Tng <ackerleytng@google.com>
Cc: pbonzini@redhat.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] KVM: guest_memfd: Drop a superfluous local var in kvm_gmem_fault_user_mapping()
Date: Fri, 10 Oct 2025 15:11:49 -0700 [thread overview]
Message-ID: <aOmEpZw7DXnuu--l@google.com> (raw)
In-Reply-To: <diqza51zhc4m.fsf@google.com>
On Thu, Oct 09, 2025, Ackerley Tng wrote:
> Sean Christopherson <seanjc@google.com> writes:
>
> > Drop the local "int err" that's buried in the middle guest_memfd's user
> > fault handler to avoid the potential for variable shadowing, e.g. if an
> > "err" variable were also declared at function scope.
> >
>
> Is the takeaway here that the variable name "err", if used, should be
> defined at function scope?
Generally speaking, for generic variables like "err", "r", and "ret", yes, because
the danger of shadowing is high, while the odds of _wanting_ to contain a return
code are low.
But as a broad rule, there's simply no "right" answer other than "it depends".
As Paolo put it, damned if you do, damned if you don't. See this thread for more:
https://lore.kernel.org/all/YZL1ZiKQVRQd8rZi@google.com
> IOW, would this code have been okay if any other variable name were
> used, like if err_folio were used instead of err?
Probably not? Because that has it's own problems. E.g. then you can end up
introducing bugs like this:
int err;
err = blah();
if (err)
goto fault_err;
folio = kvm_gmem_get_folio(inode, vmf->pgoff);
if (IS_ERR(folio)) {
int folio_err = PTR_ERR(folio);
if (folio_err == -EAGAIN)
return VM_FAULT_RETRY;
goto fault_err;
}
...
fault_err:
return vmf_error(err);
next prev parent reply other threads:[~2025-10-10 22:11 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-07 22:27 [PATCH] KVM: guest_memfd: Drop a superfluous local var in kvm_gmem_fault_user_mapping() Sean Christopherson
2025-10-09 23:46 ` Ackerley Tng
2025-10-10 22:11 ` Sean Christopherson [this message]
2025-10-10 22:33 ` Ackerley Tng
2025-10-15 18:02 ` Sean Christopherson
2025-10-20 15:51 ` Sean Christopherson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aOmEpZw7DXnuu--l@google.com \
--to=seanjc@google.com \
--cc=ackerleytng@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.