Laszlo Ersek <ler...@redhat.com> writes:
> On 02/18/19 13:56, 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);
>> }
>> break;
>> default:
>>
>
> Technically speaking, the commit message is slightly incorrect, where it
> says "all this commit does is expanding the macro" -- the "goto" is
> being removed as well.
You're right. I'll amend the commit message.
> I like the attention to detail in that you didn't add the missing
> newline character in the expanded fprintf() ;)
I wish I could claim it was intentional preservation of a bad smell as
fair warning to future readers...
> With the commit message tweaked, or not:
>
> Reviewed-by: Laszlo Ersek <ler...@redhat.com>
Thanks!
