Re: [PATCH 5/5] mm: add the __must_check attribute to {gfn,mfn}_add()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.02.2024 06:07, George Dunlap wrote:
> On Wed, Feb 14, 2024 at 7:42 PM Roger Pau Monne <roger.pau@xxxxxxxxxx> wrote:
>>
>> It's not obvious from the function itself whether the incremented value will 
>> be
>> stored in the parameter, or returned to the caller.  That has leads to bugs 
>> in
>> the past as callers assume the incremented value is stored in the parameter.
>>
>> Add the __must_check attribute to the function to easily spot callers that
>> don't consume the returned value, which signals an error in the caller logic.
>>
>> No functional change intended.
> 
> Just thinking out loud here... I wonder if "mfn_plus()" would be less
> likely to be misunderstood.  Catching this during compile is def
> better than catching it w/ Coverity or Eclair, but even better to help
> people not make the mistake in the first place.

To me while mfn_plus() would indeed be somewhat less likely to cause this
kind of confusion, I don't think that naming would entirely eliminate the
risk.

I was actually considering to add mfn_inc() for the common case of
incrementing by 1, yet I can't see an alternative name avoiding the issue
there.

Jan