On Tue, 27 Feb 2024, Jan Beulich wrote: > On 26.02.2024 23:49, Stefano Stabellini wrote: > > On Mon, 26 Feb 2024, Jan Beulich wrote: > >> On 26.02.2024 09:23, Nicola Vetrini wrote: > >>> On 2024-02-26 09:00, Jan Beulich wrote: > >>>> On 23.02.2024 23:56, Stefano Stabellini wrote: > >>>>> On Fri, 23 Feb 2024, Nicola Vetrini wrote: > >>>>>> These functions never saw a usage of their return value since > >>>>>> they were introduced, so it can be dropped since their usages > >>>>>> violate MISRA C 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 <[email protected]> > >>>>> > >>>>> Reviewed-by: Stefano Stabellini <[email protected]> > >>>> > >>>> The cleanup is certainly okay, but would one of you mind clarifying in > >>>> how > >>>> far this code is relevant for certification? I don't expect there are > >>>> plans > >>>> to run shim Xen in any projected production uses for which > >>>> certification is > >>>> relevant? (The subject prefix is also unnecessarily wide here, when > >>>> it's > >>>> only daemon code which is affected, not console code in general.) > >>>> > >>> > >>> I agree on the subject prefix being too wide. The configuration that > >>> uses consoled_guest_tx is #ifdef-ed for x86, so even in configurations > >>> that may never reach this condition this is relevant, unless its #ifdef > >>> is restricted to cases where the call may actually be reachable. > >> > >> Hmm, I see. There are contradicting goals here then: It being just X86 is > >> to reduce the risk of someone overlooking a build breakage they may > >> introduce. Whereas for certification it's quite the other way around: We'd > >> like to "hide" as much code as possible. > >> > >> Really I would have been inclined to suggest to drop the #ifdef, if > >> possible even without replacing by IS_ENABLED(), but instead leveraging > >> that pv_shim ought to be compile-time false whenever CONFIG_PV_SHIM=n. > > > > This is OK > > > > > >> After all that's a pattern we've been trying to follow. But with your > >> observation is becomes questionable whether extending use of IS_ENABLED() > >> is actually going to be helpful. Stefano - perhaps something to discuss > >> on one of the next meetings? > > > > Yes. I checked with the safety manager and his opinion is that > > IS_ENABLED() is OK to use as a way to disable code from a safety > > perspective. > > Yet unlike when #ifdef is used, such code would remain visible to e.g. > Eclair even after the preprocessing step. Note the context in which > I'm bringing this up - if IS_ENABLED() was properly used here (and as > tightly as possible), the tool would still have complained, aiui.
Let me check with Roberto about this.
