Markus Armbruster <arm...@redhat.com> writes:
> Markus Armbruster <arm...@redhat.com> writes:
>
>> Peter Maydell <peter.mayd...@linaro.org> writes:
>>
>>> 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.
>>
>> I can do that. Thanks!
>
> Double-checking... you want me to keep goto reset_flash, like this:
>
> @@ -623,8 +617,8 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
> pfl->wcycle = 0;
> pfl->status |= 0x80;
> } else {
> - DPRINTF("%s: unknown command for \"write block\"\n",
> __func__);
> - PFLASH_BUG("Write block confirm");
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "unknown command for \"write block\"\n");
> goto reset_flash;
> }
> break;
With this change:
Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
--
Alex Bennée
