Re: [PATCH] KVM: guest_memfd: Drop a superfluous local var in kvm_gmem_fault_user_mapping()

[Ackerley Tng <ackerleytng@google.com>]

Sean Christopherson <seanjc@google.com> writes:

> 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);

This is true too. Thanks, I see why it depends.