Re: [Qemu-devel] [PATCH for 3.2 v2 3/7] hw/arm/bcm2835: Use 0x prefix for hex numbers

[Peter Maydell]

On 2 November 2018 at 00:12, Philippe Mathieu-Daudé <address@hidden> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  hw/char/bcm2835_aux.c      | 2 +-
>  hw/intc/bcm2836_control.c  | 4 ++--
>  hw/misc/bcm2835_property.c | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)

Commit message says it's just adding 0x prefixes, but the
patch adds extra logging of new numbers that weren't there
before. I don't think the patch needs splitting, but the
commit message could be improved.

Also, the compile test says some of these new additions
are using the wrong kind of format string specifier.

>
> diff --git a/hw/char/bcm2835_aux.c b/hw/char/bcm2835_aux.c
> index 0364596c55..9632e8972c 100644
> --- a/hw/char/bcm2835_aux.c
> +++ b/hw/char/bcm2835_aux.c
> @@ -159,7 +159,7 @@ static void bcm2835_aux_write(void *opaque, hwaddr 
> offset, uint64_t value,
>      case AUX_ENABLES:
>          if (value != 1) {
>              qemu_log_mask(LOG_UNIMP, "%s: unsupported attempt to enable SPI "
> -                          "or disable UART\n", __func__);
> +                          "or disable UART: 0x%lx\n", __func__, value);

uint64_t needs PRIx64, not %lx.

>          }
>          break;
>
> diff --git a/hw/intc/bcm2836_control.c b/hw/intc/bcm2836_control.c
> index cfa5bc7365..96cc53f8a7 100644
> --- a/hw/intc/bcm2836_control.c
> +++ b/hw/intc/bcm2836_control.c
> @@ -204,8 +204,8 @@ static void bcm2836_control_write(void *opaque, hwaddr 
> offset,
>      } else if (offset >= REG_MBOX0_RDCLR && offset < REG_LIMIT) {
>          s->mailboxes[(offset - REG_MBOX0_RDCLR) >> 2] &= ~val;
>      } else {
> -        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %"HWADDR_PRIx"\n",
> -                      __func__, offset);
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%"HWADDR_PRIx" 
> 0x%lx\n",
> +                      __func__, offset, val);

Ditto. If you want to print the value you should also say in the
log message string what the second hex number in it is.
(Mostly we don't seem to worry about printing the data value
for "write to bogus device address" messages.)

>          return;
>      }
>
> diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c
> index 5d332324bd..c715cbaef6 100644
> --- a/hw/misc/bcm2835_property.c
> +++ b/hw/misc/bcm2835_property.c
> @@ -277,7 +277,7 @@ static void 
> bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value)
>
>          default:
>              qemu_log_mask(LOG_GUEST_ERROR,
> -                          "bcm2835_property: unhandled tag %08x\n", tag);
> +                          "bcm2835_property: unhandled tag 0x%08x\n", tag);
>              break;
>          }
>
> --
> 2.17.2

thanks
-- PMM