Re: [PATCH v2 13/24] sh/cpu: Make sure play_dead() doesn't return

From: Josh Poimboeuf
Date: Tue Feb 14 2023 - 13:28:47 EST


On Tue, Feb 14, 2023 at 08:57:39AM +0100, Philippe Mathieu-Daudé wrote:
> On 14/2/23 08:05, Josh Poimboeuf wrote:
> > play_dead() doesn't return. Make that more explicit with a BUG().
> >
> > BUG() is preferable to unreachable() because BUG() is a more explicit
> > failure mode and avoids undefined behavior like falling off the edge of
> > the function into whatever code happens to be next.
> >
> > Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> > ---
> > arch/sh/include/asm/smp-ops.h | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/sh/include/asm/smp-ops.h b/arch/sh/include/asm/smp-ops.h
> > index e27702130eb6..63866b1595a0 100644
> > --- a/arch/sh/include/asm/smp-ops.h
> > +++ b/arch/sh/include/asm/smp-ops.h
> > @@ -27,6 +27,7 @@ static inline void plat_smp_setup(void)
> > static inline void play_dead(void)
> > {
> > mp_ops->play_dead();
> > + BUG();
> > }
>
> Shouldn't we decorate plat_smp_ops::play_dead() as noreturn first?

I guess it really depends on how far we want to go down the __noreturn
rabbit hole. To keep the patch set constrained yet still useful I
stopped when I got to a function pointer, as I think it still needs a
BUG() afterwards either way.

That said, there would still be benefits of adding __noreturn to
function pointers, I just wanted to keep the patch set down to a
manageable size ;-)

--
Josh