* [Qemu-devel] [PATCH] net: cadence_gem: fix compilation error when debug is on
@ 2019-06-09 10:08 Ramon Fried
2019-06-11 8:34 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 3+ messages in thread
From: Ramon Fried @ 2019-06-09 10:08 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Jason Wang, Alistair Francis,
open list:Xilinx Zynq, Ramon Fried, Edgar E. Iglesias
defining CADENCE_GEM_ERR_DEBUG causes compilation
errors, fix that.
Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
---
hw/net/cadence_gem.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 7f63411430..5cc5a71524 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -982,8 +982,8 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
return -1;
}
- DB_PRINT("copy %d bytes to 0x%x\n", MIN(bytes_to_copy, rxbufsize),
- rx_desc_get_buffer(s->rx_desc[q]));
+ DB_PRINT("copy %d bytes to 0x%lx\n", MIN(bytes_to_copy, rxbufsize),
+ rx_desc_get_buffer(s, s->rx_desc[q]));
/* Copy packet data to emulated DMA buffer */
address_space_write(&s->dma_as, rx_desc_get_buffer(s, s->rx_desc[q]) +
@@ -1156,7 +1156,7 @@ static void gem_transmit(CadenceGEMState *s)
if (tx_desc_get_length(desc) > sizeof(tx_packet) -
(p - tx_packet)) {
DB_PRINT("TX descriptor @ 0x%x too large: size 0x%x space " \
- "0x%x\n", (unsigned)packet_desc_addr,
+ "0x%lx\n", (unsigned)packet_desc_addr,
(unsigned)tx_desc_get_length(desc),
sizeof(tx_packet) - (p - tx_packet));
break;
--
2.21.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] net: cadence_gem: fix compilation error when debug is on
2019-06-09 10:08 [Qemu-devel] [PATCH] net: cadence_gem: fix compilation error when debug is on Ramon Fried
@ 2019-06-11 8:34 ` Philippe Mathieu-Daudé
2019-06-11 11:28 ` Ramon Fried
0 siblings, 1 reply; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-11 8:34 UTC (permalink / raw)
To: Ramon Fried, qemu-devel
Cc: Peter Maydell, Jason Wang, Alistair Francis,
open list:Xilinx Zynq, Edgar E. Iglesias
Hi Ramon,
On 6/9/19 12:08 PM, Ramon Fried wrote:
> defining CADENCE_GEM_ERR_DEBUG causes compilation
> errors, fix that.
>
> Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
> ---
> hw/net/cadence_gem.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 7f63411430..5cc5a71524 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -982,8 +982,8 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
> return -1;
> }
>
> - DB_PRINT("copy %d bytes to 0x%x\n", MIN(bytes_to_copy, rxbufsize),
> - rx_desc_get_buffer(s->rx_desc[q]));
> + DB_PRINT("copy %d bytes to 0x%lx\n", MIN(bytes_to_copy, rxbufsize),
> + rx_desc_get_buffer(s, s->rx_desc[q]));
Your patch fails on 32-bit hosts:
./hw/net/cadence_gem.c:987:18: error: format '%lx' expects argument of
type 'long unsigned int', but argument 4 has type 'uint64_t {aka long
long unsigned int}' [-Werror=format=]
DB_PRINT("copy %d bytes to 0x%lx\n", MIN(bytes_to_copy, rxbufsize),
^
./hw/net/cadence_gem.c:39:24: note: in definition of macro 'DB_PRINT'
fprintf(stderr, ## __VA_ARGS__); \
^
./hw/net/cadence_gem.c: In function 'gem_transmit':
./hw/net/cadence_gem.c:1160:26: error: format '%lx' expects argument of
type 'long unsigned int', but argument 5 has type 'unsigned int'
[-Werror=format=]
DB_PRINT("TX descriptor @ 0x%x too large: size 0x%x
space " \
^
./hw/net/cadence_gem.c:39:24: note: in definition of macro 'DB_PRINT'
fprintf(stderr, ## __VA_ARGS__); \
^
cc1: all warnings being treated as errors
QEMU provides "HWADDR_PRIx" format for addresses, see for example few
lines earlier:
DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
>
> /* Copy packet data to emulated DMA buffer */
> address_space_write(&s->dma_as, rx_desc_get_buffer(s, s->rx_desc[q]) +
> @@ -1156,7 +1156,7 @@ static void gem_transmit(CadenceGEMState *s)
> if (tx_desc_get_length(desc) > sizeof(tx_packet) -
> (p - tx_packet)) {
> DB_PRINT("TX descriptor @ 0x%x too large: size 0x%x space " \
> - "0x%x\n", (unsigned)packet_desc_addr,
> + "0x%lx\n", (unsigned)packet_desc_addr,
Here the correct format seems to be "%zd" (difference of sizeof).
> (unsigned)tx_desc_get_length(desc),
> sizeof(tx_packet) - (p - tx_packet));
> break;
>
Nowadays QEMU prefers to move from the old DB_PRINT() macros to the
trace events API, see for example this commit:
https://git.qemu.org/?p=qemu.git;a=commitdiff;h=da1804d17a9ed7f060c072fbc4889db5fbc9c7d2;hp=a4f667b6714916683408b983cfe0a615a725775f
The first line you changed would be replaced by a trace event, while the
second could be replaced by a qemu_log_mask() call (it is an error
condition).
Also I suggest to include "QEMU Trivial <qemu-trivial@nongnu.org>" in
the list of recipients, so your patch might get reviewed/merged quicker.
Regards,
Phil.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] net: cadence_gem: fix compilation error when debug is on
2019-06-11 8:34 ` Philippe Mathieu-Daudé
@ 2019-06-11 11:28 ` Ramon Fried
0 siblings, 0 replies; 3+ messages in thread
From: Ramon Fried @ 2019-06-11 11:28 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Jason Wang, Alistair Francis, qemu-devel,
open list:Xilinx Zynq, Edgar E. Iglesias
On Tue, Jun 11, 2019 at 11:34 AM Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:
> Hi Ramon,
>
> On 6/9/19 12:08 PM, Ramon Fried wrote:
> > defining CADENCE_GEM_ERR_DEBUG causes compilation
> > errors, fix that.
> >
> > Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
> > ---
> > hw/net/cadence_gem.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> > index 7f63411430..5cc5a71524 100644
> > --- a/hw/net/cadence_gem.c
> > +++ b/hw/net/cadence_gem.c
> > @@ -982,8 +982,8 @@ static ssize_t gem_receive(NetClientState *nc, const
> uint8_t *buf, size_t size)
> > return -1;
> > }
> >
> > - DB_PRINT("copy %d bytes to 0x%x\n", MIN(bytes_to_copy,
> rxbufsize),
> > - rx_desc_get_buffer(s->rx_desc[q]));
> > + DB_PRINT("copy %d bytes to 0x%lx\n", MIN(bytes_to_copy,
> rxbufsize),
> > + rx_desc_get_buffer(s, s->rx_desc[q]));
>
> Your patch fails on 32-bit hosts:
>
> ./hw/net/cadence_gem.c:987:18: error: format '%lx' expects argument of
> type 'long unsigned int', but argument 4 has type 'uint64_t {aka long
> long unsigned int}' [-Werror=format=]
> DB_PRINT("copy %d bytes to 0x%lx\n", MIN(bytes_to_copy,
> rxbufsize),
> ^
> ./hw/net/cadence_gem.c:39:24: note: in definition of macro 'DB_PRINT'
> fprintf(stderr, ## __VA_ARGS__); \
> ^
> ./hw/net/cadence_gem.c: In function 'gem_transmit':
> ./hw/net/cadence_gem.c:1160:26: error: format '%lx' expects argument of
> type 'long unsigned int', but argument 5 has type 'unsigned int'
> [-Werror=format=]
> DB_PRINT("TX descriptor @ 0x%x too large: size 0x%x
> space " \
> ^
> ./hw/net/cadence_gem.c:39:24: note: in definition of macro 'DB_PRINT'
> fprintf(stderr, ## __VA_ARGS__); \
> ^
> cc1: all warnings being treated as errors
>
> QEMU provides "HWADDR_PRIx" format for addresses, see for example few
> lines earlier:
>
> DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
>
>
> >
> > /* Copy packet data to emulated DMA buffer */
> > address_space_write(&s->dma_as, rx_desc_get_buffer(s,
> s->rx_desc[q]) +
> > @@ -1156,7 +1156,7 @@ static void gem_transmit(CadenceGEMState *s)
> > if (tx_desc_get_length(desc) > sizeof(tx_packet) -
> > (p - tx_packet)) {
> > DB_PRINT("TX descriptor @ 0x%x too large: size 0x%x
> space " \
> > - "0x%x\n", (unsigned)packet_desc_addr,
> > + "0x%lx\n", (unsigned)packet_desc_addr,
>
> Here the correct format seems to be "%zd" (difference of sizeof).
>
> > (unsigned)tx_desc_get_length(desc),
> > sizeof(tx_packet) - (p - tx_packet));
> > break;
> >
>
> Nowadays QEMU prefers to move from the old DB_PRINT() macros to the
> trace events API, see for example this commit:
>
>
> https://git.qemu.org/?p=qemu.git;a=commitdiff;h=da1804d17a9ed7f060c072fbc4889db5fbc9c7d2;hp=a4f667b6714916683408b983cfe0a615a725775f
>
> The first line you changed would be replaced by a trace event, while the
> second could be replaced by a qemu_log_mask() call (it is an error
> condition).
>
> Also I suggest to include "QEMU Trivial <qemu-trivial@nongnu.org>" in
> the list of recipients, so your patch might get reviewed/merged quicker.
>
> Regards,
>
> Phil.
>
Hi Phil,
Thanks for the review, I'll send v2 soon.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-06-11 12:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-09 10:08 [Qemu-devel] [PATCH] net: cadence_gem: fix compilation error when debug is on Ramon Fried
2019-06-11 8:34 ` Philippe Mathieu-Daudé
2019-06-11 11:28 ` Ramon Fried
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.