* [Qemu-devel] [PATCH] hw: net: cadence_gem: Fix build errors in DB_PRINT() @ 2019-08-05 15:52 Bin Meng 2019-08-06 10:57 ` Stefano Garzarella 2019-08-08 4:44 ` [Qemu-devel] [PATCH v2] " Bin Meng 0 siblings, 2 replies; 9+ messages in thread From: Bin Meng @ 2019-08-05 15:52 UTC (permalink / raw) To: Edgar E. Iglesias, Jason Wang, Peter Maydell, qemu-arm, qemu-devel When CADENCE_GEM_ERR_DEBUG is turned on, there are several compilation errors in DB_PRINT(). Fix them. Signed-off-by: Bin Meng <bmeng.cn@gmail.com> --- hw/net/cadence_gem.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index d412085..7516e8f 100644 --- a/hw/net/cadence_gem.c +++ b/hw/net/cadence_gem.c @@ -983,8 +983,9 @@ 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 " TARGET_FMT_plx "\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]) + @@ -1157,7 +1158,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.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] hw: net: cadence_gem: Fix build errors in DB_PRINT() 2019-08-05 15:52 [Qemu-devel] [PATCH] hw: net: cadence_gem: Fix build errors in DB_PRINT() Bin Meng @ 2019-08-06 10:57 ` Stefano Garzarella 2019-08-08 4:45 ` Bin Meng 2019-08-08 4:44 ` [Qemu-devel] [PATCH v2] " Bin Meng 1 sibling, 1 reply; 9+ messages in thread From: Stefano Garzarella @ 2019-08-06 10:57 UTC (permalink / raw) To: Bin Meng Cc: Edgar E. Iglesias, Jason Wang, qemu-arm, qemu-devel, Peter Maydell On Mon, Aug 05, 2019 at 08:52:54AM -0700, Bin Meng wrote: > When CADENCE_GEM_ERR_DEBUG is turned on, there are several > compilation errors in DB_PRINT(). Fix them. > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > --- > > hw/net/cadence_gem.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c > index d412085..7516e8f 100644 > --- a/hw/net/cadence_gem.c > +++ b/hw/net/cadence_gem.c > @@ -983,8 +983,9 @@ 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 " TARGET_FMT_plx "\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]) + > @@ -1157,7 +1158,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, What about using 'z' modifier? I mean "0x%zx" to print sizeof(..). Thanks, Stefano ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] hw: net: cadence_gem: Fix build errors in DB_PRINT() 2019-08-06 10:57 ` Stefano Garzarella @ 2019-08-08 4:45 ` Bin Meng 0 siblings, 0 replies; 9+ messages in thread From: Bin Meng @ 2019-08-08 4:45 UTC (permalink / raw) To: Stefano Garzarella Cc: Edgar E. Iglesias, Jason Wang, qemu-arm, qemu-devel@nongnu.org Developers, Peter Maydell On Tue, Aug 6, 2019 at 6:57 PM Stefano Garzarella <sgarzare@redhat.com> wrote: > > On Mon, Aug 05, 2019 at 08:52:54AM -0700, Bin Meng wrote: > > When CADENCE_GEM_ERR_DEBUG is turned on, there are several > > compilation errors in DB_PRINT(). Fix them. > > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > > --- > > > > hw/net/cadence_gem.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c > > index d412085..7516e8f 100644 > > --- a/hw/net/cadence_gem.c > > +++ b/hw/net/cadence_gem.c > > @@ -983,8 +983,9 @@ 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 " TARGET_FMT_plx "\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]) + > > @@ -1157,7 +1158,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, > > What about using 'z' modifier? I mean "0x%zx" to print sizeof(..). Yes, good idea. Will do in v2. Thanks! Regards, Bin ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v2] hw: net: cadence_gem: Fix build errors in DB_PRINT() 2019-08-05 15:52 [Qemu-devel] [PATCH] hw: net: cadence_gem: Fix build errors in DB_PRINT() Bin Meng 2019-08-06 10:57 ` Stefano Garzarella @ 2019-08-08 4:44 ` Bin Meng 2019-08-08 5:21 ` Philippe Mathieu-Daudé 2019-08-08 9:08 ` [Qemu-devel] [Qemu-arm] " Alex Bennée 1 sibling, 2 replies; 9+ messages in thread From: Bin Meng @ 2019-08-08 4:44 UTC (permalink / raw) To: Alistair Francis, Edgar E. Iglesias, Jason Wang, Peter Maydell, Stefano Garzarella, qemu-devel, qemu-arm When CADENCE_GEM_ERR_DEBUG is turned on, there are several compilation errors in DB_PRINT(). Fix them. Signed-off-by: Bin Meng <bmeng.cn@gmail.com> --- Changes in v2: - use HWADDR_PRIx instead of TARGET_FMT_plx for consistency - use 'z' modifier to print sizeof(..) hw/net/cadence_gem.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index d412085..b6ff2c1 100644 --- a/hw/net/cadence_gem.c +++ b/hw/net/cadence_gem.c @@ -983,8 +983,9 @@ 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%" HWADDR_PRIx "\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]) + @@ -1157,7 +1158,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%zx\n", (unsigned)packet_desc_addr, (unsigned)tx_desc_get_length(desc), sizeof(tx_packet) - (p - tx_packet)); break; -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2] hw: net: cadence_gem: Fix build errors in DB_PRINT() 2019-08-08 4:44 ` [Qemu-devel] [PATCH v2] " Bin Meng @ 2019-08-08 5:21 ` Philippe Mathieu-Daudé 2019-08-08 6:36 ` Bin Meng 2019-08-08 9:08 ` [Qemu-devel] [Qemu-arm] " Alex Bennée 1 sibling, 1 reply; 9+ messages in thread From: Philippe Mathieu-Daudé @ 2019-08-08 5:21 UTC (permalink / raw) To: Bin Meng, Alistair Francis, Edgar E. Iglesias, Jason Wang, Peter Maydell, Stefano Garzarella, qemu-devel, qemu-arm Hi, On 8/8/19 6:44 AM, Bin Meng wrote: > When CADENCE_GEM_ERR_DEBUG is turned on, there are several > compilation errors in DB_PRINT(). Fix them. > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > > --- > > Changes in v2: Please don't reply to previous version, post as a new thread (it is harder to notice your new versions in a emails threaded view). > - use HWADDR_PRIx instead of TARGET_FMT_plx for consistency > - use 'z' modifier to print sizeof(..) > > hw/net/cadence_gem.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c > index d412085..b6ff2c1 100644 > --- a/hw/net/cadence_gem.c > +++ b/hw/net/cadence_gem.c > @@ -983,8 +983,9 @@ 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%" HWADDR_PRIx "\n", rx_desc_get_buffer() returns a uint64_t, shouldn't you use a PRIx64 format here? > + MIN(bytes_to_copy, rxbufsize), Nitpick #1: since you are cleaning this file up, bytes_to_copy and rxbufsize are both unsigned, so the first format should be %u instead of %d. > + 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]) + > @@ -1157,7 +1158,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%zx\n", (unsigned)packet_desc_addr, Nitpick #2: packet_desc_addr is of type hwaddr, so removing the cast the 1st format is HWADDR_PRIx, also removing the 2nd cast the 2nd format is PRIx64. Then the 3rd format is now correct. > (unsigned)tx_desc_get_length(desc), > sizeof(tx_packet) - (p - tx_packet)); > break; > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2] hw: net: cadence_gem: Fix build errors in DB_PRINT() 2019-08-08 5:21 ` Philippe Mathieu-Daudé @ 2019-08-08 6:36 ` Bin Meng 2019-08-08 7:01 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 9+ messages in thread From: Bin Meng @ 2019-08-08 6:36 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Peter Maydell, Jason Wang, qemu-devel@nongnu.org Developers, qemu-arm, Alistair Francis, Edgar E. Iglesias, Stefano Garzarella On Thu, Aug 8, 2019 at 1:21 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > Hi, > > On 8/8/19 6:44 AM, Bin Meng wrote: > > When CADENCE_GEM_ERR_DEBUG is turned on, there are several > > compilation errors in DB_PRINT(). Fix them. > > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > > > > --- > > > > Changes in v2: > > Please don't reply to previous version, post as a new thread (it is > harder to notice your new versions in a emails threaded view). > OK. > > - use HWADDR_PRIx instead of TARGET_FMT_plx for consistency > > - use 'z' modifier to print sizeof(..) > > > > hw/net/cadence_gem.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c > > index d412085..b6ff2c1 100644 > > --- a/hw/net/cadence_gem.c > > +++ b/hw/net/cadence_gem.c > > @@ -983,8 +983,9 @@ 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%" HWADDR_PRIx "\n", > > rx_desc_get_buffer() returns a uint64_t, shouldn't you use a PRIx64 > format here? HWADDR_PRIx expands to PRIx64. I got your point that since it does not return hwaddr, so we should use PRIx64 directly. Correct? > > > + MIN(bytes_to_copy, rxbufsize), > > Nitpick #1: since you are cleaning this file up, bytes_to_copy and > rxbufsize are both unsigned, so the first format should be %u instead of %d. Sure, will do in v3. > > > + 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]) + > > @@ -1157,7 +1158,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%zx\n", (unsigned)packet_desc_addr, > > Nitpick #2: packet_desc_addr is of type hwaddr, so removing the cast the > 1st format is HWADDR_PRIx, also removing the 2nd cast the 2nd format is > PRIx64. packet_desc_addr() return unsigned, so %x should be OK. > > Then the 3rd format is now correct. > > > (unsigned)tx_desc_get_length(desc), > > sizeof(tx_packet) - (p - tx_packet)); > > break; > > Regards, Bin ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2] hw: net: cadence_gem: Fix build errors in DB_PRINT() 2019-08-08 6:36 ` Bin Meng @ 2019-08-08 7:01 ` Philippe Mathieu-Daudé 2019-08-08 7:18 ` Bin Meng 0 siblings, 1 reply; 9+ messages in thread From: Philippe Mathieu-Daudé @ 2019-08-08 7:01 UTC (permalink / raw) To: Bin Meng Cc: Peter Maydell, Jason Wang, qemu-devel@nongnu.org Developers, qemu-arm, Alistair Francis, Edgar E. Iglesias, Stefano Garzarella On 8/8/19 8:36 AM, Bin Meng wrote: > On Thu, Aug 8, 2019 at 1:21 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: >> >> Hi, >> >> On 8/8/19 6:44 AM, Bin Meng wrote: >>> When CADENCE_GEM_ERR_DEBUG is turned on, there are several >>> compilation errors in DB_PRINT(). Fix them. >>> >>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>> >>> --- >>> >>> Changes in v2: >> >> Please don't reply to previous version, post as a new thread (it is >> harder to notice your new versions in a emails threaded view). >> > > OK. > >>> - use HWADDR_PRIx instead of TARGET_FMT_plx for consistency >>> - use 'z' modifier to print sizeof(..) >>> >>> hw/net/cadence_gem.c | 7 ++++--- >>> 1 file changed, 4 insertions(+), 3 deletions(-) >>> >>> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c >>> index d412085..b6ff2c1 100644 >>> --- a/hw/net/cadence_gem.c >>> +++ b/hw/net/cadence_gem.c >>> @@ -983,8 +983,9 @@ 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%" HWADDR_PRIx "\n", >> >> rx_desc_get_buffer() returns a uint64_t, shouldn't you use a PRIx64 >> format here? > > HWADDR_PRIx expands to PRIx64. I got your point that since it does not > return hwaddr, so we should use PRIx64 directly. Correct? > >> >>> + MIN(bytes_to_copy, rxbufsize), >> >> Nitpick #1: since you are cleaning this file up, bytes_to_copy and >> rxbufsize are both unsigned, so the first format should be %u instead of %d. > > Sure, will do in v3. > >> >>> + 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]) + >>> @@ -1157,7 +1158,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%zx\n", (unsigned)packet_desc_addr, >> >> Nitpick #2: packet_desc_addr is of type hwaddr, so removing the cast the >> 1st format is HWADDR_PRIx, also removing the 2nd cast the 2nd format is >> PRIx64. > > packet_desc_addr() return unsigned, so %x should be OK. 'packet_desc_addr' is of type hwaddr, '(unsigned)packet_desc_addr' is casted to type unsigned. Anyhow I now remember I already reviewed this patch: https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg03263.html >> >> Then the 3rd format is now correct. >> >>> (unsigned)tx_desc_get_length(desc), >>> sizeof(tx_packet) - (p - tx_packet)); >>> break; >>> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2] hw: net: cadence_gem: Fix build errors in DB_PRINT() 2019-08-08 7:01 ` Philippe Mathieu-Daudé @ 2019-08-08 7:18 ` Bin Meng 0 siblings, 0 replies; 9+ messages in thread From: Bin Meng @ 2019-08-08 7:18 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Peter Maydell, Jason Wang, qemu-devel@nongnu.org Developers, qemu-arm, Alistair Francis, Edgar E. Iglesias, Stefano Garzarella On Thu, Aug 8, 2019 at 3:01 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > On 8/8/19 8:36 AM, Bin Meng wrote: > > On Thu, Aug 8, 2019 at 1:21 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > >> > >> Hi, > >> > >> On 8/8/19 6:44 AM, Bin Meng wrote: > >>> When CADENCE_GEM_ERR_DEBUG is turned on, there are several > >>> compilation errors in DB_PRINT(). Fix them. > >>> > >>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > >>> > >>> --- > >>> > >>> Changes in v2: > >> > >> Please don't reply to previous version, post as a new thread (it is > >> harder to notice your new versions in a emails threaded view). > >> > > > > OK. > > > >>> - use HWADDR_PRIx instead of TARGET_FMT_plx for consistency > >>> - use 'z' modifier to print sizeof(..) > >>> > >>> hw/net/cadence_gem.c | 7 ++++--- > >>> 1 file changed, 4 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c > >>> index d412085..b6ff2c1 100644 > >>> --- a/hw/net/cadence_gem.c > >>> +++ b/hw/net/cadence_gem.c > >>> @@ -983,8 +983,9 @@ 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%" HWADDR_PRIx "\n", > >> > >> rx_desc_get_buffer() returns a uint64_t, shouldn't you use a PRIx64 > >> format here? > > > > HWADDR_PRIx expands to PRIx64. I got your point that since it does not > > return hwaddr, so we should use PRIx64 directly. Correct? > > > >> > >>> + MIN(bytes_to_copy, rxbufsize), > >> > >> Nitpick #1: since you are cleaning this file up, bytes_to_copy and > >> rxbufsize are both unsigned, so the first format should be %u instead of %d. > > > > Sure, will do in v3. > > > >> > >>> + 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]) + > >>> @@ -1157,7 +1158,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%zx\n", (unsigned)packet_desc_addr, > >> > >> Nitpick #2: packet_desc_addr is of type hwaddr, so removing the cast the > >> 1st format is HWADDR_PRIx, also removing the 2nd cast the 2nd format is > >> PRIx64. > > > > packet_desc_addr() return unsigned, so %x should be OK. > > 'packet_desc_addr' is of type hwaddr, Ah, a typo! I meant to say: tx_desc_get_length() returns unsigned, so just removing the 2nd cast is correct. But you wanted to change to PRIx64 which I don't understand. > '(unsigned)packet_desc_addr' is casted to type unsigned. > > Anyhow I now remember I already reviewed this patch: > https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg03263.html > OK, looks the same issue was discovered by someone else :) Regards, Bin ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [Qemu-arm] [PATCH v2] hw: net: cadence_gem: Fix build errors in DB_PRINT() 2019-08-08 4:44 ` [Qemu-devel] [PATCH v2] " Bin Meng 2019-08-08 5:21 ` Philippe Mathieu-Daudé @ 2019-08-08 9:08 ` Alex Bennée 1 sibling, 0 replies; 9+ messages in thread From: Alex Bennée @ 2019-08-08 9:08 UTC (permalink / raw) To: qemu-arm Cc: Peter Maydell, Jason Wang, qemu-devel, Alistair Francis, Edgar E. Iglesias, Stefano Garzarella Bin Meng <bmeng.cn@gmail.com> writes: > When CADENCE_GEM_ERR_DEBUG is turned on, there are several > compilation errors in DB_PRINT(). Fix them. The first fix should be to ensure the format strings are validated in normal compilation. This can be achieved by allowing the compiler to optimise away debug strings with constant folding... for example: #define tlb_debug(fmt, ...) do { \ if (DEBUG_TLB_LOG_GATE) { \ qemu_log_mask(CPU_LOG_MMU, "%s: " fmt, __func__, \ ## __VA_ARGS__); \ } else if (DEBUG_TLB_GATE) { \ fprintf(stderr, "%s: " fmt, __func__, ## __VA_ARGS__); \ } \ } while (0) However ultimately most debug printfs are either a) stale leftovers from original development or b) could be considered for conversion to tracepoints. > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > > --- > > Changes in v2: > - use HWADDR_PRIx instead of TARGET_FMT_plx for consistency > - use 'z' modifier to print sizeof(..) > > hw/net/cadence_gem.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c > index d412085..b6ff2c1 100644 > --- a/hw/net/cadence_gem.c > +++ b/hw/net/cadence_gem.c > @@ -983,8 +983,9 @@ 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%" HWADDR_PRIx "\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]) + > @@ -1157,7 +1158,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%zx\n", (unsigned)packet_desc_addr, > (unsigned)tx_desc_get_length(desc), > sizeof(tx_packet) - (p - tx_packet)); > break; -- Alex Bennée ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-08-08 9:09 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-05 15:52 [Qemu-devel] [PATCH] hw: net: cadence_gem: Fix build errors in DB_PRINT() Bin Meng 2019-08-06 10:57 ` Stefano Garzarella 2019-08-08 4:45 ` Bin Meng 2019-08-08 4:44 ` [Qemu-devel] [PATCH v2] " Bin Meng 2019-08-08 5:21 ` Philippe Mathieu-Daudé 2019-08-08 6:36 ` Bin Meng 2019-08-08 7:01 ` Philippe Mathieu-Daudé 2019-08-08 7:18 ` Bin Meng 2019-08-08 9:08 ` [Qemu-devel] [Qemu-arm] " Alex Bennée
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).