On Tue, 27 Feb 2024, Jan Beulich wrote: > On 27.02.2024 12:52, Julien Grall wrote: > > Hi Jan, > > > > On 27/02/2024 07:28, Jan Beulich wrote: > >> On 27.02.2024 01:26, Stefano Stabellini wrote: > >>> On Mon, 26 Feb 2024, Jan Beulich wrote: > >>>> On 23.02.2024 10:35, Nicola Vetrini wrote: > >>>>> Refactor cpu_notifier_call_chain into two functions: > >>>>> - the variant that is allowed to fail loses the nofail flag > >>>>> - the variant that shouldn't fail is encapsulated in a call > >>>>> to the failing variant, with an additional check. > >>>>> > >>>>> This prevents uses of the function that are not supposed to > >>>>> fail from ignoring the return value, thus violating Rule 17.7: > >>>>> "The value returned by a function having non-void return type shall > >>>>> be used". > >>>>> > >>>>> No functional change. > >>>>> > >>>>> Signed-off-by: Nicola Vetrini <nicola.vetr...@bugseng.com> > >>>> > >>>> I'm afraid I disagree with this kind of bifurcation. No matter what > >>>> Misra thinks or says, it is normal for return values of functions to > >>>> not always be relevant to check. > >>> > >>> Hi Jan, I disagree. > >>> > >>> Regardless of MISRA, I really think return values need to be checked. > >>> Moreover, we decided as a group to honor MISRA Rule 17.7, which requires > >>> return values to be checked. This patch is a good step forward. > >> > >> Yet splitting functions isn't the only way to deal with Misra's > >> requirements, I suppose. After all there are functions where the > >> return value is purely courtesy for perhaps just one of its callers. > > > > You are right that we have some places where one caller care about the > > return value. But the problem is how do you tell whether the return was > > ignored on purpose or not? > > > > We had at least one XSA because the return value of a function was not > > checked (see XSA-222). We also had plenty of smaller patches to check > > returns. > > > > So far, we added __must_check when we believed return values should be > > checked. But usually at the point we notice, this is far too late. > > > > To me the goal should be that we enforce __must_check everywhere. We are > > probably going to detect places where we forgot to check the return. For > > thoses that are on purpose, we can document them. > > > >> > >> Splitting simply doesn't scale very well, imo. > > > > Do you have another proposal? As Stefano said, we adopted the rule 17.7. > > So we know need a solution to address it. > > One possibility that was circulated while discussing was to add (void) > casts. I'm not a huge fan of those, but between the two options that > might be the lesser evil. We also use funny (should I say ugly) > workarounds in a few cases where we have __must_check but still want > to not really handle the return value in certain cases. Given there are > example in the code base, extending use of such constructs is certainly > also something that may want considering.
I asked Roberto if void casts are an option for compliance. In any case, I don't think we should use void casts in the specific cases this patch is dealing with. Void casts (if anything) should be a last resort while this patch fixes the issue in a better way.
