[PATCH V2 1/2] lib: sbi: add some macros to detect BUG at runtime

[Mitchell Horne]

On Wed, Sep 15, 2021 at 10:56 AM Xiang W <wxjstz at 126.com> wrote:
>
> 在 2021-09-15星期三的 20:39 +0800，杜东写道：
> >
> >
> > ----- Original Message -----
> > > From: "Xiang W" <wxjstz at 126.com>
> > > To: "opensbi" <opensbi at lists.infradead.org>
> > > Cc: "atish patra" <atish.patra at wdc.com>, "anup patel" <
> > > anup.patel at wdc.com>, "Xiang W" <wxjstz at 126.com>
> > > Sent: Wednesday, September 15, 2021 5:03:29 PM
> > > Subject: [PATCH V2 1/2] lib: sbi: add some macros to detect BUG at
> > > runtime
> >
> > > Two macros are mainly added. One is called BUG(), which is used to
> > > put
> > > in unreachable branches. One named BUG_ON, used for assertion.
> > >
> > > Signed-off-by: Xiang W <wxjstz at 126.com>
> > > ---
> > > include/sbi/sbi_console.h | 13 +++++++++++++
> > > 1 file changed, 13 insertions(+)
> > >
> > > diff --git a/include/sbi/sbi_console.h b/include/sbi/sbi_console.h
> > > index e24ba5f..e75a279 100644
> > > --- a/include/sbi/sbi_console.h
> > > +++ b/include/sbi/sbi_console.h
> > > @@ -11,6 +11,7 @@
> > > #define __SBI_CONSOLE_H__
> > >
> > > #include <sbi/sbi_types.h>
> > > +#include <sbi/riscv_asm.h>
> > >
> > > struct sbi_console_device {
> > >         /** Name of the console device */
> > > @@ -51,4 +52,16 @@ struct sbi_scratch;
> > >
> > > int sbi_console_init(struct sbi_scratch *scratch);
> > >
> > > +#define BUG() do { \
> > > +       sbi_printf("BUG: failure at %s:%d/%s()!\n", __FILE__,
> > > __LINE__, __func__); \
> > > +       while (1)       \
> > > +               wfi();  \
> > > +       __builtin_unreachable(); \
> > > +} while (0)
> > > +
> > > +#define BUG_ON(cond) do { \
> > > +       if (!(cond))    \
> > > +               BUG(); \
> > > +} while (0)
> > > +
> >
> > If the BUG_ON has a similar semantics as BUG_ON in Linux, it should
> > be:
> >   + if (cond)   \
> >   +             BUG(); \
> >
> I want to implement BUG_ON like assert. If the meaning of linux is like
> this, I think it can be used as a reference
>

Maybe it should be named ASSERT or SBI_ASSERT then? It does not seem
like your other patch even uses this however.

In my opinion, BUG() and BUG_ON() are confusing names to begin with;
they do not obviously describe their semantics. If you insist on using
these names, their behaviour should match Linux.

Cheers,
Mitchell


> Regards，
> Xiang W
> > > #endif
> > > --
> > > 2.30.2
> > >
> > >
> > > --
> > > opensbi mailing list
> > > opensbi at lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/opensbi
> >
> > Regards,
> > Dong
> >
>
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi