On 19.02.2024 06:07, George Dunlap wrote: > On Wed, Feb 14, 2024 at 7:42 PM Roger Pau Monne <[email protected]> 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
