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

[Xiang W]

在 2021-09-15星期三的 15:49 +0100，Jessica Clarke写道：
> On 15 Sep 2021, at 15:41, Mitchell Horne <mhorne at freebsd.org> wrote:
> > 
> > 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.
> 
> Also BUG is a bit of a lazy interface; line numbers change over time,
> really you should be using some kind of panic function that takes a
> format string with a meaningful message. That also gives the user
> some
> hope of knowing what went wrong and maybe working around or fixing
> it.
> And why not reuse sbi_hart_hang rather than inlining another copy of
> it?
Thanks for the suggestion

Regards，
Xiang W
> 
> Jess