[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is
From: |
Alex Bennée |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand |
Date: |
Thu, 21 Feb 2019 15:08:15 +0000 |
User-agent: |
mu4e 1.1.0; emacs 26.1 |
Markus Armbruster <address@hidden> writes:
> Markus Armbruster <address@hidden> writes:
>
>> Peter Maydell <address@hidden> writes:
>>
>>> 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.
>>
>> 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 <address@hidden>
--
Alex Bennée
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, (continued)
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Peter Maydell, 2019/02/21
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Laszlo Ersek, 2019/02/21
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Peter Maydell, 2019/02/21
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Laszlo Ersek, 2019/02/21
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Philippe Mathieu-Daudé, 2019/02/21
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Laszlo Ersek, 2019/02/21
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Philippe Mathieu-Daudé, 2019/02/21
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Markus Armbruster, 2019/02/21
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Alex Bennée, 2019/02/21
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand, Markus Armbruster, 2019/02/22
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 02/10] pflash: Macro PFLASH_BUG() is used just once, expand,
Alex Bennée <=
[Qemu-ppc] [PATCH 03/10] hw: Use CFI_PFLASH0{1, 2} and TYPE_CFI_PFLASH0{1, 2}, Markus Armbruster, 2019/02/18
- Re: [Qemu-ppc] [PATCH 03/10] hw: Use CFI_PFLASH0{1, 2} and TYPE_CFI_PFLASH0{1, 2}, Laszlo Ersek, 2019/02/18
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 03/10] hw: Use CFI_PFLASH0{1, 2} and TYPE_CFI_PFLASH0{1, 2}, Philippe Mathieu-Daudé, 2019/02/19
- Re: [Qemu-ppc] [PATCH 03/10] hw: Use CFI_PFLASH0{1, 2} and TYPE_CFI_PFLASH0{1, 2}, Alex Bennée, 2019/02/21
[Qemu-ppc] [PATCH 10/10] hw/arm hw/xtensa: De-duplicate pflash creation code some, Markus Armbruster, 2019/02/18