diff mbox

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

Message ID CAEgOgz43hxy8OkChNyq5kk=ueP_9i+AmjC_p9Pw94JWPQT=rOg@mail.gmail.com
State New
Headers show

Commit Message

Peter Crosthwaite Feb. 20, 2013, 2:19 a.m. UTC 
On Tue, Feb 19, 2013 at 6:35 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   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.

Done, thanks for the correction. Do you have a lightweight test vector
that exercises this? The iovec stuff in my areas is all limited to
single iovec element.  Interdiff below.

Regards,
Peter

When calling hexdump for each iovec element the buffer offsets
> printed are wrong for all but the first iovec element.
>
> cheers,
>   Gerd
>
>

Comments

Gerd Hoffmann Feb. 20, 2013, 9:04 a.m. UTC | #1 
Hi,

> Done, thanks for the correction. Do you have a lightweight test vector
> that exercises this? The iovec stuff in my areas is all limited to
> single iovec element.  Interdiff below.

hw/usb/dev-network.c (with TRAFFC_DEBUG) calls this on usb packets which
could trigger this in theory.  Depends on how the guest allocates the
usb packet buffers though, so I'm not fully sure whenever that actually
triggers in practice.

> @@ -202,11 +202,16 @@ void iov_hexdump(const struct iovec *iov, const
> unsigned int iov_cnt,
>                   FILE *fp, const char *prefix, size_t limit)
>  {
>      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;
> +    size_t size = 0;
> +    char *buf;
> +
> +    for (v = 0; v < iov_cnt; v++) {
> +        size += iov[v].iov_len;
>      }
> +    size = size > limit ? limit : size;
> +    buf = g_malloc(size);
> +    iov_to_buf(iov, iov_cnt, 0, buf, size);

You've lost the actual hexdump() call here ;)

> +    g_free(buf);

Otherwise it looks fine.

cheers,
  Gerd
diff mbox

Patch

diff --git a/util/hexdump.c b/util/hexdump.c
index 0bf0f38..0d0efc8 100644
--- a/util/hexdump.c
+++ b/util/hexdump.c
@@ -1,12 +1,11 @@ 
 /*
  * Helper to hexdump a buffer
  *
+ * Copyright (c) 2013 Red Hat, Inc.
+ * Copyright (c) 2013 Gerd Hoffmann <kraxel@redhat.com>
  * Copyright (c) 2013 Peter Crosthwaite <peter.crosthwaite@xilinx.com>
  * Copyright (c) 2013 Xilinx, Inc
  *
- * Based on git commit 3a1dca94d6dba00fe0fd4c4a28449f57e01b9b6c
- * Author: Gerd Hoffmann <kraxel@redhat.com>
- *
  * This work is licensed under the terms of the GNU GPL, version 2.  See
  * the COPYING file in the top-level directory.
  *
diff --git a/util/iov.c b/util/iov.c
index 91d79ae..99f0b50 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -202,11 +202,16 @@  void iov_hexdump(const struct iovec *iov, const
unsigned int iov_cnt,
                  FILE *fp, const char *prefix, size_t limit)
 {
     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;
+    size_t size = 0;
+    char *buf;
+
+    for (v = 0; v < iov_cnt; v++) {
+        size += iov[v].iov_len;
     }
+    size = size > limit ? limit : size;
+    buf = g_malloc(size);
+    iov_to_buf(iov, iov_cnt, 0, buf, size);
+    g_free(buf);
 }