Patchwork [v9,1/3] iov: Factor out hexdumper

login
register
mail settings
Submitter Peter Crosthwaite
Date Feb. 8, 2013, 3:42 a.m.
Message ID <076e2799-0389-4bc2-ae53-717a9f89206a@TX2EHSMHS042.ehs.local>
Download mbox | patch
Permalink /patch/219035/
State New
Headers show

Comments

Peter Crosthwaite - Feb. 8, 2013, 3:42 a.m.
Factor out the hexdumper functionality from iov for all to use. Useful for
creating verbose debug printfery that dumps packet data.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 include/qemu/hexdump.h |    3 +++
 util/Makefile.objs     |    1 +
 util/hexdump.c         |   45 +++++++++++++++++++++++++++++++++++++++++++++
 util/iov.c             |   31 ++++++-------------------------
 4 files changed, 55 insertions(+), 25 deletions(-)
 create mode 100644 include/qemu/hexdump.h
 create mode 100644 util/hexdump.c
Michael Tokarev - Feb. 8, 2013, 8:42 a.m.
08.02.2013 07:42, Peter Crosthwaite wrote:
> Factor out the hexdumper functionality from iov for all to use. Useful for
> creating verbose debug printfery that dumps packet data.

Two comments below.

> diff --git a/include/qemu/hexdump.h b/include/qemu/hexdump.h
> new file mode 100644
> index 0000000..87bccf4
> --- /dev/null
> +++ b/include/qemu/hexdump.h
> @@ -0,0 +1,3 @@
> +#include "qemu-common.h"
> +
> +void hexdump(const char *buf, FILE *fp, const char *prefix, size_t size);

Do we really need this include, for just one function declaration?
I'd put it stright to qemu-common.h (lacking an "util" header).


> diff --git a/util/hexdump.c b/util/hexdump.c
> new file mode 100644
> index 0000000..74df358
> --- /dev/null
> +++ b/util/hexdump.c
> @@ -0,0 +1,45 @@
> +/*
> + * Helper to hexdump a buffer
> + *
> + * Copyright (c) 2013 Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> + * Copyright (c) 2013 Xilinx, Inc
> + *
> + * Factored out from iov.c:
> + * Copyright IBM, Corp. 2007, 2008
> + * Copyright (C) 2010 Red Hat, Inc.
> + *
> + * Author(s):
> + *  Anthony Liguori <aliguori@us.ibm.com>
> + *  Amit Shah <amit.shah@redhat.com>
> + *  Michael Tokarev <mjt@tls.msk.ru>

My (c) was for other parts of iov, not this tiny hexdump
function, so it's okay to remove it from there :)

Thanks!

/mjt
Peter Crosthwaite - Feb. 19, 2013, 6:16 a.m.
Hi Michael,

On Fri, Feb 8, 2013 at 6:42 PM, Michael Tokarev <mjt@tls.msk.ru> wrote:
> 08.02.2013 07:42, Peter Crosthwaite wrote:
>> Factor out the hexdumper functionality from iov for all to use. Useful for
>> creating verbose debug printfery that dumps packet data.
>
> Two comments below.
>
>> diff --git a/include/qemu/hexdump.h b/include/qemu/hexdump.h
>> new file mode 100644
>> index 0000000..87bccf4
>> --- /dev/null
>> +++ b/include/qemu/hexdump.h
>> @@ -0,0 +1,3 @@
>> +#include "qemu-common.h"
>> +
>> +void hexdump(const char *buf, FILE *fp, const char *prefix, size_t size);
>
> Do we really need this include, for just one function declaration?
> I'd put it stright to qemu-common.h (lacking an "util" header).
>
>

Done

>> diff --git a/util/hexdump.c b/util/hexdump.c
>> new file mode 100644
>> index 0000000..74df358
>> --- /dev/null
>> +++ b/util/hexdump.c
>> @@ -0,0 +1,45 @@
>> +/*
>> + * Helper to hexdump a buffer
>> + *
>> + * Copyright (c) 2013 Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> + * Copyright (c) 2013 Xilinx, Inc
>> + *
>> + * Factored out from iov.c:
>> + * Copyright IBM, Corp. 2007, 2008
>> + * Copyright (C) 2010 Red Hat, Inc.
>> + *
>> + * Author(s):
>> + *  Anthony Liguori <aliguori@us.ibm.com>
>> + *  Amit Shah <amit.shah@redhat.com>
>> + *  Michael Tokarev <mjt@tls.msk.ru>
>
> My (c) was for other parts of iov, not this tiny hexdump
> function, so it's okay to remove it from there :)
>

I ran git blame on it and its actually Gerds code. Gerd you want
co-authorship and the (c) of this hexdump.c?

commit 3a1dca94d6dba00fe0fd4c4a28449f57e01b9b6c
Author: Gerd Hoffmann <kraxel@redhat.com>
Date:   Tue Jul 12 13:35:10 2011 +0200

    Add iov_hexdump()

    Useful for debugging purposes.

    Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>


> Thanks!
>
> /mjt
>
Gerd Hoffmann - Feb. 19, 2013, 8:35 a.m.
Hi,

> I ran git blame on it and its actually Gerds code. Gerd you want
> co-authorship and the (c) of this hexdump.c?

Yes.

And don't break iov_hexdump() behavior please.  I suggest to just
iov_to_buf() into a temporary buffer, then pass that to a single hexdump
call.  When calling hexdump for each iovec element the buffer offsets
printed are wrong for all but the first iovec element.

cheers,
  Gerd

Patch

diff --git a/include/qemu/hexdump.h b/include/qemu/hexdump.h
new file mode 100644
index 0000000..87bccf4
--- /dev/null
+++ b/include/qemu/hexdump.h
@@ -0,0 +1,3 @@ 
+#include "qemu-common.h"
+
+void hexdump(const char *buf, FILE *fp, const char *prefix, size_t size);
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 495a178..068ceac 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -8,3 +8,4 @@  util-obj-y += error.o qemu-error.o
 util-obj-$(CONFIG_POSIX) += compatfd.o
 util-obj-y += iov.o aes.o qemu-config.o qemu-sockets.o uri.o notify.o
 util-obj-y += qemu-option.o qemu-progress.o
+util-obj-y += hexdump.o
diff --git a/util/hexdump.c b/util/hexdump.c
new file mode 100644
index 0000000..74df358
--- /dev/null
+++ b/util/hexdump.c
@@ -0,0 +1,45 @@ 
+/*
+ * Helper to hexdump a buffer
+ *
+ * Copyright (c) 2013 Peter Crosthwaite <peter.crosthwaite@xilinx.com>
+ * Copyright (c) 2013 Xilinx, Inc
+ *
+ * Factored out from iov.c:
+ * Copyright IBM, Corp. 2007, 2008
+ * Copyright (C) 2010 Red Hat, Inc.
+ *
+ * Author(s):
+ *  Anthony Liguori <aliguori@us.ibm.com>
+ *  Amit Shah <amit.shah@redhat.com>
+ *  Michael Tokarev <mjt@tls.msk.ru>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#include "qemu-common.h"
+#include "qemu/hexdump.h"
+
+void hexdump(const char *buf, FILE *fp, const char *prefix, size_t size)
+{
+    unsigned int b;
+
+    for (b = 0; b < size; b++) {
+        if ((b % 16) == 0) {
+            fprintf(fp, "%s: %04x:", prefix, b);
+        }
+        if ((b % 4) == 0) {
+            fprintf(fp, " ");
+        }
+        fprintf(fp, " %02x", (unsigned char)buf[b]);
+        if ((b % 16) == 15) {
+            fprintf(fp, "\n");
+        }
+    }
+    if ((b % 16) != 0) {
+        fprintf(fp, "\n");
+    }
+}
diff --git a/util/iov.c b/util/iov.c
index fbe675d..d89933d 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -17,6 +17,7 @@ 
  */
 
 #include "qemu/iov.h"
+#include "qemu/hexdump.h"
 
 #ifdef _WIN32
 # include <windows.h>
@@ -201,31 +202,11 @@  ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt,
 void iov_hexdump(const struct iovec *iov, const unsigned int iov_cnt,
                  FILE *fp, const char *prefix, size_t limit)
 {
-    unsigned int i, v, b;
-    uint8_t *c;
-
-    c = iov[0].iov_base;
-    for (i = 0, v = 0, b = 0; b < limit; i++, b++) {
-        if (i == iov[v].iov_len) {
-            i = 0; v++;
-            if (v == iov_cnt) {
-                break;
-            }
-            c = iov[v].iov_base;
-        }
-        if ((b % 16) == 0) {
-            fprintf(fp, "%s: %04x:", prefix, b);
-        }
-        if ((b % 4) == 0) {
-            fprintf(fp, " ");
-        }
-        fprintf(fp, " %02x", c[i]);
-        if ((b % 16) == 15) {
-            fprintf(fp, "\n");
-        }
-    }
-    if ((b % 16) != 0) {
-        fprintf(fp, "\n");
+    int v;
+    for (v = 0; v < iov_cnt && limit; v++) {
+        int size = limit < iov[v].iov_len ? limit : iov[v].iov_len;
+        hexdump(iov[v].iov_base, fp, prefix, size);
+        limit -= size;
     }
 }