Re: [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand

[Peter Maydell]

On Tue, 19 Feb 2019 at 12:41, Philippe Mathieu-Daudé <address@hidden> wrote:
>
> On 2/18/19 1:56 PM, Markus Armbruster wrote:
> > PFLASH_BUG()'s lone use has a suspicious smell: it prints "Possible
> > BUG", which sounds like a warning, then calls exit(1), followed by
> > unreachable goto reset_flash.  All this commit does is expanding the
> > macro, so the smell becomes more poignant, and the macro can be
> > deleted.
> >
> > Signed-off-by: Markus Armbruster <address@hidden>
> > ---
> >  hw/block/pflash_cfi01.c | 10 ++--------
> >  1 file changed, 2 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> > index 9efa7aa9af..f73c30a3ee 100644
> > --- a/hw/block/pflash_cfi01.c
> > +++ b/hw/block/pflash_cfi01.c
> > @@ -49,12 +49,6 @@
> >  #include "sysemu/sysemu.h"
> >  #include "trace.h"
> >
> > -#define PFLASH_BUG(fmt, ...) \
> > -do { \
> > -    fprintf(stderr, "PFLASH: Possible BUG - " fmt, ## __VA_ARGS__); \
> > -    exit(1); \
> > -} while(0)
> > -
> >  /* #define PFLASH_DEBUG */
> >  #ifdef PFLASH_DEBUG
> >  #define DPRINTF(fmt, ...)                                   \
> > @@ -624,8 +618,8 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr 
> > offset,
> >                  pfl->status |= 0x80;
> >              } else {
> >                  DPRINTF("%s: unknown command for \"write block\"\n", 
> > __func__);
> > -                PFLASH_BUG("Write block confirm");
> > -                goto reset_flash;
> > +                fprintf(stderr, "PFLASH: Possible BUG - Write block 
> > confirm");
> > +                exit(1);
>
> Don't you want to use hw_error here?
>
>       hw_error("PFLASH: Possible BUG - Write block confirm");

This should just be
   qemu_log_mask(LOG_GUEST_ERROR, ...);
(replacing both the DPRINTF and the PFLASH_BUG()).

It's triggerable by a guest (if it puts the device into write-block
mode and then feeds it a bogus command byte), so it's just a guest
error, not an issue with our model of the pflash.

thanks
-- PMM