| Message ID | 20190609100837.17586-1-rfried.dev@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | net: cadence_gem: fix compilation error when debug is on | expand |
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.
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.
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;
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(-)