Re: [PATCH v2 2/2] util/hexdump: Reorder qemu_hexdump() arguments

2020-09-11 Thread Laurent Vivier
Le 22/08/2020 à 20:09, Philippe Mathieu-Daudé a écrit :
> qemu_hexdump()'s pointer to the buffer and length of the
> buffer are closely related arguments but are widely separated
> in the argument list order (also, the format of 
> function prototypes is usually to have the FILE* argument
> coming first).
> 
> Reorder the arguments as "fp, prefix, buf, size" which is
> more logical.
> 
> Suggested-by: Peter Maydell 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  include/qemu-common.h|  4 ++--
>  hw/dma/xlnx_dpdma.c  |  2 +-
>  hw/net/fsl_etsec/etsec.c |  2 +-
>  hw/net/fsl_etsec/rings.c |  2 +-
>  hw/sd/sd.c   |  2 +-
>  hw/usb/redirect.c|  2 +-
>  net/colo-compare.c   | 24 
>  net/net.c|  2 +-
>  util/hexdump.c   |  4 ++--
>  util/iov.c   |  2 +-
>  10 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index 6834883822f..9cfd62669bf 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -138,8 +138,8 @@ int os_parse_cmd_args(int index, const char *optarg);
>   * Hexdump a buffer to a file. An optional string prefix is added to every 
> line
>   */
>  
> -void qemu_hexdump(const void *bufptr, FILE *fp,
> -  const char *prefix, size_t size);
> +void qemu_hexdump(FILE *fp, const char *prefix,
> +  const void *bufptr, size_t size);
>  
>  /*
>   * helper to parse debug environment variables
> diff --git a/hw/dma/xlnx_dpdma.c b/hw/dma/xlnx_dpdma.c
> index d75a8069426..967548abd31 100644
> --- a/hw/dma/xlnx_dpdma.c
> +++ b/hw/dma/xlnx_dpdma.c
> @@ -388,7 +388,7 @@ static void xlnx_dpdma_dump_descriptor(DPDMADescriptor 
> *desc)
>  {
>  if (DEBUG_DPDMA) {
>  qemu_log("DUMP DESCRIPTOR:\n");
> -qemu_hexdump(desc, stdout, "", sizeof(DPDMADescriptor));
> +qemu_hexdump(stdout, "", desc, sizeof(DPDMADescriptor));
>  }
>  }
>  
> diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
> index c817a28decd..c5edb25dc9f 100644
> --- a/hw/net/fsl_etsec/etsec.c
> +++ b/hw/net/fsl_etsec/etsec.c
> @@ -357,7 +357,7 @@ static ssize_t etsec_receive(NetClientState *nc,
>  
>  #if defined(HEX_DUMP)
>  fprintf(stderr, "%s receive size:%zd\n", nc->name, size);
> -qemu_hexdump(buf, stderr, "", size);
> +qemu_hexdump(stderr, "", buf, size);
>  #endif
>  /* Flush is unnecessary as are already in receiving path */
>  etsec->need_flush = false;
> diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
> index 337a55fc957..628648a9c37 100644
> --- a/hw/net/fsl_etsec/rings.c
> +++ b/hw/net/fsl_etsec/rings.c
> @@ -269,7 +269,7 @@ static void process_tx_bd(eTSEC *etsec,
>  
>  #if defined(HEX_DUMP)
>  qemu_log("eTSEC Send packet size:%d\n", etsec->tx_buffer_len);
> -qemu_hexdump(etsec->tx_buffer, stderr, "", etsec->tx_buffer_len);
> +qemu_hexdump(stderr, "", etsec->tx_buffer, etsec->tx_buffer_len);
>  #endif  /* ETSEC_RING_DEBUG */
>  
>  if (etsec->first_bd.flags & BD_TX_TOEUN) {
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 190e4cf1232..6508939b1f4 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1781,7 +1781,7 @@ send_response:
>  }
>  
>  #ifdef DEBUG_SD
> -qemu_hexdump(response, stderr, "Response", rsplen);
> +qemu_hexdump(stderr, "Response", response, rsplen);
>  #endif
>  
>  return rsplen;
> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> index 09edb0d81c0..48f38d33912 100644
> --- a/hw/usb/redirect.c
> +++ b/hw/usb/redirect.c
> @@ -240,7 +240,7 @@ static void usbredir_log_data(USBRedirDevice *dev, const 
> char *desc,
>  if (dev->debug < usbredirparser_debug_data) {
>  return;
>  }
> -qemu_hexdump(data, stderr, desc, len);
> +qemu_hexdump(stderr, desc, data, len);
>  }
>  
>  /*
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 550272b3baa..4a5ed642e9a 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -494,10 +494,10 @@ sec:
>  g_queue_push_head(>secondary_list, spkt);
>  
>  if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
> -qemu_hexdump(ppkt->data, stderr,
> -"colo-compare ppkt", ppkt->size);
> -qemu_hexdump(spkt->data, stderr,
> -"colo-compare spkt", spkt->size);
> +qemu_hexdump(stderr, "colo-compare ppkt",
> + ppkt->data, ppkt->size);
> +qemu_hexdump(stderr, "colo-compare spkt",
> + spkt->data, spkt->size);
>  }
>  
>  colo_compare_inconsistency_notify(s);
> @@ -535,10 +535,10 @@ static int colo_packet_compare_udp(Packet *spkt, Packet 
> *ppkt)
>  trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
>  trace_colo_compare_udp_miscompare("Secondary pkt size", spkt->size);
>  if 

Re: [PATCH v2 2/2] util/hexdump: Reorder qemu_hexdump() arguments

2020-08-28 Thread Edgar E. Iglesias
On Sat, Aug 22, 2020 at 08:09:50PM +0200, Philippe Mathieu-Daudé wrote:
> qemu_hexdump()'s pointer to the buffer and length of the
> buffer are closely related arguments but are widely separated
> in the argument list order (also, the format of 
> function prototypes is usually to have the FILE* argument
> coming first).
> 
> Reorder the arguments as "fp, prefix, buf, size" which is
> more logical.
> 
> Suggested-by: Peter Maydell 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Edgar E. Iglesias 



> ---
>  include/qemu-common.h|  4 ++--
>  hw/dma/xlnx_dpdma.c  |  2 +-
>  hw/net/fsl_etsec/etsec.c |  2 +-
>  hw/net/fsl_etsec/rings.c |  2 +-
>  hw/sd/sd.c   |  2 +-
>  hw/usb/redirect.c|  2 +-
>  net/colo-compare.c   | 24 
>  net/net.c|  2 +-
>  util/hexdump.c   |  4 ++--
>  util/iov.c   |  2 +-
>  10 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index 6834883822f..9cfd62669bf 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -138,8 +138,8 @@ int os_parse_cmd_args(int index, const char *optarg);
>   * Hexdump a buffer to a file. An optional string prefix is added to every 
> line
>   */
>  
> -void qemu_hexdump(const void *bufptr, FILE *fp,
> -  const char *prefix, size_t size);
> +void qemu_hexdump(FILE *fp, const char *prefix,
> +  const void *bufptr, size_t size);
>  
>  /*
>   * helper to parse debug environment variables
> diff --git a/hw/dma/xlnx_dpdma.c b/hw/dma/xlnx_dpdma.c
> index d75a8069426..967548abd31 100644
> --- a/hw/dma/xlnx_dpdma.c
> +++ b/hw/dma/xlnx_dpdma.c
> @@ -388,7 +388,7 @@ static void xlnx_dpdma_dump_descriptor(DPDMADescriptor 
> *desc)
>  {
>  if (DEBUG_DPDMA) {
>  qemu_log("DUMP DESCRIPTOR:\n");
> -qemu_hexdump(desc, stdout, "", sizeof(DPDMADescriptor));
> +qemu_hexdump(stdout, "", desc, sizeof(DPDMADescriptor));
>  }
>  }
>  
> diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
> index c817a28decd..c5edb25dc9f 100644
> --- a/hw/net/fsl_etsec/etsec.c
> +++ b/hw/net/fsl_etsec/etsec.c
> @@ -357,7 +357,7 @@ static ssize_t etsec_receive(NetClientState *nc,
>  
>  #if defined(HEX_DUMP)
>  fprintf(stderr, "%s receive size:%zd\n", nc->name, size);
> -qemu_hexdump(buf, stderr, "", size);
> +qemu_hexdump(stderr, "", buf, size);
>  #endif
>  /* Flush is unnecessary as are already in receiving path */
>  etsec->need_flush = false;
> diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
> index 337a55fc957..628648a9c37 100644
> --- a/hw/net/fsl_etsec/rings.c
> +++ b/hw/net/fsl_etsec/rings.c
> @@ -269,7 +269,7 @@ static void process_tx_bd(eTSEC *etsec,
>  
>  #if defined(HEX_DUMP)
>  qemu_log("eTSEC Send packet size:%d\n", etsec->tx_buffer_len);
> -qemu_hexdump(etsec->tx_buffer, stderr, "", etsec->tx_buffer_len);
> +qemu_hexdump(stderr, "", etsec->tx_buffer, etsec->tx_buffer_len);
>  #endif  /* ETSEC_RING_DEBUG */
>  
>  if (etsec->first_bd.flags & BD_TX_TOEUN) {
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 190e4cf1232..6508939b1f4 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1781,7 +1781,7 @@ send_response:
>  }
>  
>  #ifdef DEBUG_SD
> -qemu_hexdump(response, stderr, "Response", rsplen);
> +qemu_hexdump(stderr, "Response", response, rsplen);
>  #endif
>  
>  return rsplen;
> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> index 09edb0d81c0..48f38d33912 100644
> --- a/hw/usb/redirect.c
> +++ b/hw/usb/redirect.c
> @@ -240,7 +240,7 @@ static void usbredir_log_data(USBRedirDevice *dev, const 
> char *desc,
>  if (dev->debug < usbredirparser_debug_data) {
>  return;
>  }
> -qemu_hexdump(data, stderr, desc, len);
> +qemu_hexdump(stderr, desc, data, len);
>  }
>  
>  /*
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 550272b3baa..4a5ed642e9a 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -494,10 +494,10 @@ sec:
>  g_queue_push_head(>secondary_list, spkt);
>  
>  if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
> -qemu_hexdump(ppkt->data, stderr,
> -"colo-compare ppkt", ppkt->size);
> -qemu_hexdump(spkt->data, stderr,
> -"colo-compare spkt", spkt->size);
> +qemu_hexdump(stderr, "colo-compare ppkt",
> + ppkt->data, ppkt->size);
> +qemu_hexdump(stderr, "colo-compare spkt",
> + spkt->data, spkt->size);
>  }
>  
>  colo_compare_inconsistency_notify(s);
> @@ -535,10 +535,10 @@ static int colo_packet_compare_udp(Packet *spkt, Packet 
> *ppkt)
>  trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
>  trace_colo_compare_udp_miscompare("Secondary pkt 

[PATCH v2 2/2] util/hexdump: Reorder qemu_hexdump() arguments

2020-08-22 Thread Philippe Mathieu-Daudé
qemu_hexdump()'s pointer to the buffer and length of the
buffer are closely related arguments but are widely separated
in the argument list order (also, the format of 
function prototypes is usually to have the FILE* argument
coming first).

Reorder the arguments as "fp, prefix, buf, size" which is
more logical.

Suggested-by: Peter Maydell 
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/qemu-common.h|  4 ++--
 hw/dma/xlnx_dpdma.c  |  2 +-
 hw/net/fsl_etsec/etsec.c |  2 +-
 hw/net/fsl_etsec/rings.c |  2 +-
 hw/sd/sd.c   |  2 +-
 hw/usb/redirect.c|  2 +-
 net/colo-compare.c   | 24 
 net/net.c|  2 +-
 util/hexdump.c   |  4 ++--
 util/iov.c   |  2 +-
 10 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index 6834883822f..9cfd62669bf 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -138,8 +138,8 @@ int os_parse_cmd_args(int index, const char *optarg);
  * Hexdump a buffer to a file. An optional string prefix is added to every line
  */
 
-void qemu_hexdump(const void *bufptr, FILE *fp,
-  const char *prefix, size_t size);
+void qemu_hexdump(FILE *fp, const char *prefix,
+  const void *bufptr, size_t size);
 
 /*
  * helper to parse debug environment variables
diff --git a/hw/dma/xlnx_dpdma.c b/hw/dma/xlnx_dpdma.c
index d75a8069426..967548abd31 100644
--- a/hw/dma/xlnx_dpdma.c
+++ b/hw/dma/xlnx_dpdma.c
@@ -388,7 +388,7 @@ static void xlnx_dpdma_dump_descriptor(DPDMADescriptor 
*desc)
 {
 if (DEBUG_DPDMA) {
 qemu_log("DUMP DESCRIPTOR:\n");
-qemu_hexdump(desc, stdout, "", sizeof(DPDMADescriptor));
+qemu_hexdump(stdout, "", desc, sizeof(DPDMADescriptor));
 }
 }
 
diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
index c817a28decd..c5edb25dc9f 100644
--- a/hw/net/fsl_etsec/etsec.c
+++ b/hw/net/fsl_etsec/etsec.c
@@ -357,7 +357,7 @@ static ssize_t etsec_receive(NetClientState *nc,
 
 #if defined(HEX_DUMP)
 fprintf(stderr, "%s receive size:%zd\n", nc->name, size);
-qemu_hexdump(buf, stderr, "", size);
+qemu_hexdump(stderr, "", buf, size);
 #endif
 /* Flush is unnecessary as are already in receiving path */
 etsec->need_flush = false;
diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
index 337a55fc957..628648a9c37 100644
--- a/hw/net/fsl_etsec/rings.c
+++ b/hw/net/fsl_etsec/rings.c
@@ -269,7 +269,7 @@ static void process_tx_bd(eTSEC *etsec,
 
 #if defined(HEX_DUMP)
 qemu_log("eTSEC Send packet size:%d\n", etsec->tx_buffer_len);
-qemu_hexdump(etsec->tx_buffer, stderr, "", etsec->tx_buffer_len);
+qemu_hexdump(stderr, "", etsec->tx_buffer, etsec->tx_buffer_len);
 #endif  /* ETSEC_RING_DEBUG */
 
 if (etsec->first_bd.flags & BD_TX_TOEUN) {
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 190e4cf1232..6508939b1f4 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1781,7 +1781,7 @@ send_response:
 }
 
 #ifdef DEBUG_SD
-qemu_hexdump(response, stderr, "Response", rsplen);
+qemu_hexdump(stderr, "Response", response, rsplen);
 #endif
 
 return rsplen;
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 09edb0d81c0..48f38d33912 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -240,7 +240,7 @@ static void usbredir_log_data(USBRedirDevice *dev, const 
char *desc,
 if (dev->debug < usbredirparser_debug_data) {
 return;
 }
-qemu_hexdump(data, stderr, desc, len);
+qemu_hexdump(stderr, desc, data, len);
 }
 
 /*
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 550272b3baa..4a5ed642e9a 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -494,10 +494,10 @@ sec:
 g_queue_push_head(>secondary_list, spkt);
 
 if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
-qemu_hexdump(ppkt->data, stderr,
-"colo-compare ppkt", ppkt->size);
-qemu_hexdump(spkt->data, stderr,
-"colo-compare spkt", spkt->size);
+qemu_hexdump(stderr, "colo-compare ppkt",
+ ppkt->data, ppkt->size);
+qemu_hexdump(stderr, "colo-compare spkt",
+ spkt->data, spkt->size);
 }
 
 colo_compare_inconsistency_notify(s);
@@ -535,10 +535,10 @@ static int colo_packet_compare_udp(Packet *spkt, Packet 
*ppkt)
 trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
 trace_colo_compare_udp_miscompare("Secondary pkt size", spkt->size);
 if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
-qemu_hexdump(ppkt->data, stderr, "colo-compare pri pkt",
- ppkt->size);
-qemu_hexdump(spkt->data, stderr, "colo-compare sec pkt",
- spkt->size);
+qemu_hexdump(stderr, "colo-compare pri