On Tue, 19 Feb 2019 at 12:41, Philippe Mathieu-Daudé <phi...@redhat.com> 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 <arm...@redhat.com>
> > ---
> > 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
