* [Qemu-devel] [PATCH] hw/dma/xilinx_axidma: debug printf fixups @ 2016-01-05 13:22 Andrew Jones 2016-01-05 14:07 ` Eric Blake 0 siblings, 1 reply; 5+ messages in thread From: Andrew Jones @ 2016-01-05 13:22 UTC (permalink / raw) To: qemu-devel; +Cc: edgar.iglesias, crosthwaite.peter, alistair.francis (Found by grepping for broken PRI users.) Signed-off-by: Andrew Jones <drjones@redhat.com> --- hw/dma/xilinx_axidma.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c index b1cfa11356a26..2ab0772cd19ae 100644 --- a/hw/dma/xilinx_axidma.c +++ b/hw/dma/xilinx_axidma.c @@ -180,10 +180,10 @@ static inline int streamid_from_addr(hwaddr addr) #ifdef DEBUG_ENET static void stream_desc_show(struct SDesc *d) { - qemu_log("buffer_addr = " PRIx64 "\n", d->buffer_address); - qemu_log("nxtdesc = " PRIx64 "\n", d->nxtdesc); - qemu_log("control = %x\n", d->control); - qemu_log("status = %x\n", d->status); + qemu_log("buffer_addr = 0x%" PRIx64 "\n", d->buffer_address); + qemu_log("nxtdesc = 0x%" PRIx64 "\n", d->nxtdesc); + qemu_log("control = 0x%x\n", d->control); + qemu_log("status = 0x%x\n", d->status); } #endif -- 2.4.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/dma/xilinx_axidma: debug printf fixups 2016-01-05 13:22 [Qemu-devel] [PATCH] hw/dma/xilinx_axidma: debug printf fixups Andrew Jones @ 2016-01-05 14:07 ` Eric Blake 2016-01-05 15:32 ` Andrew Jones 0 siblings, 1 reply; 5+ messages in thread From: Eric Blake @ 2016-01-05 14:07 UTC (permalink / raw) To: Andrew Jones, qemu-devel Cc: edgar.iglesias, alistair.francis, crosthwaite.peter [-- Attachment #1: Type: text/plain, Size: 1315 bytes --] On 01/05/2016 06:22 AM, Andrew Jones wrote: > (Found by grepping for broken PRI users.) > > Signed-off-by: Andrew Jones <drjones@redhat.com> > --- > hw/dma/xilinx_axidma.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c > index b1cfa11356a26..2ab0772cd19ae 100644 > --- a/hw/dma/xilinx_axidma.c > +++ b/hw/dma/xilinx_axidma.c > @@ -180,10 +180,10 @@ static inline int streamid_from_addr(hwaddr addr) > #ifdef DEBUG_ENET > static void stream_desc_show(struct SDesc *d) > { > - qemu_log("buffer_addr = " PRIx64 "\n", d->buffer_address); > - qemu_log("nxtdesc = " PRIx64 "\n", d->nxtdesc); > - qemu_log("control = %x\n", d->control); > - qemu_log("status = %x\n", d->status); > + qemu_log("buffer_addr = 0x%" PRIx64 "\n", d->buffer_address); > + qemu_log("nxtdesc = 0x%" PRIx64 "\n", d->nxtdesc); > + qemu_log("control = 0x%x\n", d->control); > + qemu_log("status = 0x%x\n", d->status); This is dead code. Nothing uses stream_desc_show() even when DEBUG_ENET is defined. I'd just delete the function and #ifdef altogether, instead. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/dma/xilinx_axidma: debug printf fixups 2016-01-05 14:07 ` Eric Blake @ 2016-01-05 15:32 ` Andrew Jones 2016-01-06 1:45 ` Alistair Francis 0 siblings, 1 reply; 5+ messages in thread From: Andrew Jones @ 2016-01-05 15:32 UTC (permalink / raw) To: Eric Blake Cc: edgar.iglesias, alistair.francis, qemu-devel, crosthwaite.peter On Tue, Jan 05, 2016 at 07:07:22AM -0700, Eric Blake wrote: > On 01/05/2016 06:22 AM, Andrew Jones wrote: > > (Found by grepping for broken PRI users.) > > > > Signed-off-by: Andrew Jones <drjones@redhat.com> > > --- > > hw/dma/xilinx_axidma.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c > > index b1cfa11356a26..2ab0772cd19ae 100644 > > --- a/hw/dma/xilinx_axidma.c > > +++ b/hw/dma/xilinx_axidma.c > > @@ -180,10 +180,10 @@ static inline int streamid_from_addr(hwaddr addr) > > #ifdef DEBUG_ENET > > static void stream_desc_show(struct SDesc *d) > > { > > - qemu_log("buffer_addr = " PRIx64 "\n", d->buffer_address); > > - qemu_log("nxtdesc = " PRIx64 "\n", d->nxtdesc); > > - qemu_log("control = %x\n", d->control); > > - qemu_log("status = %x\n", d->status); > > + qemu_log("buffer_addr = 0x%" PRIx64 "\n", d->buffer_address); > > + qemu_log("nxtdesc = 0x%" PRIx64 "\n", d->nxtdesc); > > + qemu_log("control = 0x%x\n", d->control); > > + qemu_log("status = 0x%x\n", d->status); > > This is dead code. Nothing uses stream_desc_show() even when DEBUG_ENET > is defined. I'd just delete the function and #ifdef altogether, instead. Sounds good, but I guess I'll leave the keep+fix vs. throw decision to the maintainers, rather than to submit a v2 ripping it out. Thanks, drew > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/dma/xilinx_axidma: debug printf fixups 2016-01-05 15:32 ` Andrew Jones @ 2016-01-06 1:45 ` Alistair Francis 2016-01-06 12:51 ` Andrew Jones 0 siblings, 1 reply; 5+ messages in thread From: Alistair Francis @ 2016-01-06 1:45 UTC (permalink / raw) To: Andrew Jones Cc: Edgar Iglesias, Peter Crosthwaite, qemu-devel@nongnu.org Developers, Alistair Francis On Tue, Jan 5, 2016 at 7:32 AM, Andrew Jones <drjones@redhat.com> wrote: > On Tue, Jan 05, 2016 at 07:07:22AM -0700, Eric Blake wrote: >> On 01/05/2016 06:22 AM, Andrew Jones wrote: >> > (Found by grepping for broken PRI users.) >> > >> > Signed-off-by: Andrew Jones <drjones@redhat.com> >> > --- >> > hw/dma/xilinx_axidma.c | 8 ++++---- >> > 1 file changed, 4 insertions(+), 4 deletions(-) >> > >> > diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c >> > index b1cfa11356a26..2ab0772cd19ae 100644 >> > --- a/hw/dma/xilinx_axidma.c >> > +++ b/hw/dma/xilinx_axidma.c >> > @@ -180,10 +180,10 @@ static inline int streamid_from_addr(hwaddr addr) >> > #ifdef DEBUG_ENET >> > static void stream_desc_show(struct SDesc *d) >> > { >> > - qemu_log("buffer_addr = " PRIx64 "\n", d->buffer_address); >> > - qemu_log("nxtdesc = " PRIx64 "\n", d->nxtdesc); >> > - qemu_log("control = %x\n", d->control); >> > - qemu_log("status = %x\n", d->status); >> > + qemu_log("buffer_addr = 0x%" PRIx64 "\n", d->buffer_address); >> > + qemu_log("nxtdesc = 0x%" PRIx64 "\n", d->nxtdesc); >> > + qemu_log("control = 0x%x\n", d->control); >> > + qemu_log("status = 0x%x\n", d->status); >> >> This is dead code. Nothing uses stream_desc_show() even when DEBUG_ENET >> is defined. I'd just delete the function and #ifdef altogether, instead. > > Sounds good, but I guess I'll leave the keep+fix vs. throw decision to the > maintainers, rather than to submit a v2 ripping it out. I don't see any reason to keep dead code around. I think it should be removed. If you send a V2 removing it (or a new patch altogether) I'll review it. Thanks, Alistair > > Thanks, > drew > >> >> -- >> Eric Blake eblake redhat com +1-919-301-3266 >> Libvirt virtualization library http://libvirt.org >> > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/dma/xilinx_axidma: debug printf fixups 2016-01-06 1:45 ` Alistair Francis @ 2016-01-06 12:51 ` Andrew Jones 0 siblings, 0 replies; 5+ messages in thread From: Andrew Jones @ 2016-01-06 12:51 UTC (permalink / raw) To: Alistair Francis Cc: Edgar Iglesias, qemu-devel@nongnu.org Developers, Peter Crosthwaite On Tue, Jan 05, 2016 at 05:45:57PM -0800, Alistair Francis wrote: > On Tue, Jan 5, 2016 at 7:32 AM, Andrew Jones <drjones@redhat.com> wrote: > > On Tue, Jan 05, 2016 at 07:07:22AM -0700, Eric Blake wrote: > >> On 01/05/2016 06:22 AM, Andrew Jones wrote: > >> > (Found by grepping for broken PRI users.) > >> > > >> > Signed-off-by: Andrew Jones <drjones@redhat.com> > >> > --- > >> > hw/dma/xilinx_axidma.c | 8 ++++---- > >> > 1 file changed, 4 insertions(+), 4 deletions(-) > >> > > >> > diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c > >> > index b1cfa11356a26..2ab0772cd19ae 100644 > >> > --- a/hw/dma/xilinx_axidma.c > >> > +++ b/hw/dma/xilinx_axidma.c > >> > @@ -180,10 +180,10 @@ static inline int streamid_from_addr(hwaddr addr) > >> > #ifdef DEBUG_ENET > >> > static void stream_desc_show(struct SDesc *d) > >> > { > >> > - qemu_log("buffer_addr = " PRIx64 "\n", d->buffer_address); > >> > - qemu_log("nxtdesc = " PRIx64 "\n", d->nxtdesc); > >> > - qemu_log("control = %x\n", d->control); > >> > - qemu_log("status = %x\n", d->status); > >> > + qemu_log("buffer_addr = 0x%" PRIx64 "\n", d->buffer_address); > >> > + qemu_log("nxtdesc = 0x%" PRIx64 "\n", d->nxtdesc); > >> > + qemu_log("control = 0x%x\n", d->control); > >> > + qemu_log("status = 0x%x\n", d->status); > >> > >> This is dead code. Nothing uses stream_desc_show() even when DEBUG_ENET > >> is defined. I'd just delete the function and #ifdef altogether, instead. > > > > Sounds good, but I guess I'll leave the keep+fix vs. throw decision to the > > maintainers, rather than to submit a v2 ripping it out. > > I don't see any reason to keep dead code around. I think it should be removed. The reason I see, is that this function could be useful to temporarily add it different places while debugging. I.e. this is no different than a collection of temporary printf's, but saves the time of rewriting those printf's whenever/wherever they're necessary. I suspect that's why this function is here in the first place. That said, I don't debug this file, so I don't really have any say on whether or not it's of any use now. Anyway, based on the fact this function has PRI bugs in it that would break compilation, I guess nobody debugs this file with DEBUG_ENET turned on. > > If you send a V2 removing it (or a new patch altogether) I'll review it. OK, sending a new patch that kills it. Thanks, drew ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-01-06 12:51 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-01-05 13:22 [Qemu-devel] [PATCH] hw/dma/xilinx_axidma: debug printf fixups Andrew Jones 2016-01-05 14:07 ` Eric Blake 2016-01-05 15:32 ` Andrew Jones 2016-01-06 1:45 ` Alistair Francis 2016-01-06 12:51 ` Andrew Jones
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).