Le 15/04/2024 à 17:35, Arnd Bergmann a écrit : > On Mon, Apr 15, 2024, at 04:19, Michael Ellerman wrote: >> "Arnd Bergmann" <arnd@xxxxxxxx> writes: >>> On Thu, Apr 11, 2024, at 11:27, Adrian Hunter wrote: >>>> On 11/04/24 11:22, Christophe Leroy wrote: >>>> >>>> That is fragile because it depends on defined(__OPTIMIZE__), >>>> so it should still be: >>> >>> If there is a function that is defined but that must never be >>> called, I think we are doing something wrong. >> >> It's a pretty inevitable result of using IS_ENABLED(), which the docs >> encourage people to use. > > Using IS_ENABLED() is usually a good idea, as it helps avoid > adding extra #ifdef checks and just drops static functions as > dead code, or lets you call extern functions that are conditionally > defined in a different file. > > The thing is that here it does not do either of those and > adds more complexity than it avoids. > >> In this case it could easily be turned into a build error by just making >> it an extern rather than a static inline. >> >> But I think Christophe's solution is actually better, because it's more >> explicit, ie. this function should not be called and if it is that's a >> build time error. > > I haven't seen a good solution here. Ideally we'd just define > the functions unconditionally and have IS_ENABLED() take care > of letting the compiler drop them silently, but that doesn't > build because of missing struct members. > > I won't object to either an 'extern' declaration or the > 'BUILD_BUG_ON()' if you and others prefer that, both are better > than BUG() here. I still think my suggestion would be a little > simpler. The advantage of the BUILD_BUG() against the extern is that the error gets detected at buildtime. With the extern it gets detected only at link-time. But agree with you, the missing struct members defeats the advantages of IS_ENABLED(). At the end, how many instances of struct timekeeper do we have in the system ? With a quick look I see only two instances: tkcore.timekeeper and shadow_timekeeper. If I'm correct, wouldn't it just be simpler to have the three debug struct members defined at all time ? Christophe
