All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] util: Improved qemu_hexmap() to include an ascii dump of the buffer
@ 2016-03-22  7:41 Isaac Lozano
  2016-03-23  0:14 ` John Snow
  2016-03-23  8:11 ` Thomas Huth
  0 siblings, 2 replies; 4+ messages in thread
From: Isaac Lozano @ 2016-03-22  7:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Isaac Lozano

qemu_hexdump() in util/hexdump.c has been changed to give also include a
ascii dump of the buffer. Also, calls to hex_dump() in net/net.c have
been replaced with calls to qemu_hexdump(). This takes care of two misc
BiteSized Tasks.

Signed-off-by: Isaac Lozano <109lozanoi@gmail.com>
---
 net/net.c      | 31 +------------------------------
 util/hexdump.c | 35 ++++++++++++++++++++++++-----------
 2 files changed, 25 insertions(+), 41 deletions(-)

diff --git a/net/net.c b/net/net.c
index 1a78edf..8d51ffb 100644
--- a/net/net.c
+++ b/net/net.c
@@ -79,34 +79,6 @@ int default_net = 1;
 /***********************************************************/
 /* network device redirectors */
 
-#if defined(DEBUG_NET)
-static void hex_dump(FILE *f, const uint8_t *buf, int size)
-{
-    int len, i, j, c;
-
-    for(i=0;i<size;i+=16) {
-        len = size - i;
-        if (len > 16)
-            len = 16;
-        fprintf(f, "%08x ", i);
-        for(j=0;j<16;j++) {
-            if (j < len)
-                fprintf(f, " %02x", buf[i+j]);
-            else
-                fprintf(f, "   ");
-        }
-        fprintf(f, " ");
-        for(j=0;j<len;j++) {
-            c = buf[i+j];
-            if (c < ' ' || c > '~')
-                c = '.';
-            fprintf(f, "%c", c);
-        }
-        fprintf(f, "\n");
-    }
-}
-#endif
-
 static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
 {
     const char *p, *p1;
@@ -661,8 +633,7 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender,
     int ret;
 
 #ifdef DEBUG_NET
-    printf("qemu_send_packet_async:\n");
-    hex_dump(stdout, buf, size);
+    qemu_hexdump((const char*)buf, stdout, "qemu_send_packet_async", size);
 #endif
 
     if (sender->link_down || !sender->peer) {
diff --git a/util/hexdump.c b/util/hexdump.c
index 1d9c129..886e53c 100644
--- a/util/hexdump.c
+++ b/util/hexdump.c
@@ -18,21 +18,34 @@
 
 void qemu_hexdump(const char *buf, FILE *fp, const char *prefix, size_t size)
 {
-    unsigned int b;
+    unsigned int b, len, i, c;
 
-    for (b = 0; b < size; b++) {
-        if ((b % 16) == 0) {
-            fprintf(fp, "%s: %04x:", prefix, b);
+    for (b = 0; b < size; b += 16) {
+        len = size - b;
+        if (len > 16) {
+            len = 16;
         }
-        if ((b % 4) == 0) {
-            fprintf(fp, " ");
+        fprintf(fp, "%s: %04x:", prefix, b);
+        for (i = 0; i < 16; i++) {
+            if ((i % 4) == 0) {
+                fprintf(fp, " ");
+            }
+            if (i < len) {
+                fprintf(fp, " %02x", (unsigned char)buf[b + i]);
+            }
+            else
+            {
+                fprintf(fp, "   ");
+            }
         }
-        fprintf(fp, " %02x", (unsigned char)buf[b]);
-        if ((b % 16) == 15) {
-            fprintf(fp, "\n");
+        fprintf(fp, " ");
+        for (i = 0; i < len; i++) {
+            c = buf[b+i];
+            if (c < ' ' || c > '~') {
+                c = '.';
+            }
+            fprintf(fp, "%c", c);
         }
-    }
-    if ((b % 16) != 0) {
         fprintf(fp, "\n");
     }
 }
-- 
2.7.2

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] util: Improved qemu_hexmap() to include an ascii dump of the buffer
  2016-03-22  7:41 [Qemu-devel] [PATCH] util: Improved qemu_hexmap() to include an ascii dump of the buffer Isaac Lozano
@ 2016-03-23  0:14 ` John Snow
  2016-03-23  8:04   ` Thomas Huth
  2016-03-23  8:11 ` Thomas Huth
  1 sibling, 1 reply; 4+ messages in thread
From: John Snow @ 2016-03-23  0:14 UTC (permalink / raw)
  To: Isaac Lozano; +Cc: Thomas Huth, Jason Wang, qemu-devel



On 03/22/2016 03:41 AM, Isaac Lozano wrote:
> qemu_hexdump() in util/hexdump.c has been changed to give also include a
> ascii dump of the buffer. Also, calls to hex_dump() in net/net.c have
> been replaced with calls to qemu_hexdump(). This takes care of two misc
> BiteSized Tasks.
> 
> Signed-off-by: Isaac Lozano <109lozanoi@gmail.com>
> ---
>  net/net.c      | 31 +------------------------------
>  util/hexdump.c | 35 ++++++++++++++++++++++++-----------
>  2 files changed, 25 insertions(+), 41 deletions(-)
> 
> diff --git a/net/net.c b/net/net.c
> index 1a78edf..8d51ffb 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -79,34 +79,6 @@ int default_net = 1;
>  /***********************************************************/
>  /* network device redirectors */
>  
> -#if defined(DEBUG_NET)
> -static void hex_dump(FILE *f, const uint8_t *buf, int size)
> -{
> -    int len, i, j, c;
> -
> -    for(i=0;i<size;i+=16) {
> -        len = size - i;
> -        if (len > 16)
> -            len = 16;
> -        fprintf(f, "%08x ", i);
> -        for(j=0;j<16;j++) {
> -            if (j < len)
> -                fprintf(f, " %02x", buf[i+j]);
> -            else
> -                fprintf(f, "   ");
> -        }
> -        fprintf(f, " ");
> -        for(j=0;j<len;j++) {
> -            c = buf[i+j];
> -            if (c < ' ' || c > '~')
> -                c = '.';
> -            fprintf(f, "%c", c);
> -        }
> -        fprintf(f, "\n");
> -    }
> -}
> -#endif
> -
>  static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
>  {
>      const char *p, *p1;
> @@ -661,8 +633,7 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender,
>      int ret;
>  
>  #ifdef DEBUG_NET
> -    printf("qemu_send_packet_async:\n");
> -    hex_dump(stdout, buf, size);
> +    qemu_hexdump((const char*)buf, stdout, "qemu_send_packet_async", size);
>  #endif
>  
>      if (sender->link_down || !sender->peer) {
> diff --git a/util/hexdump.c b/util/hexdump.c
> index 1d9c129..886e53c 100644
> --- a/util/hexdump.c
> +++ b/util/hexdump.c
> @@ -18,21 +18,34 @@
>  
>  void qemu_hexdump(const char *buf, FILE *fp, const char *prefix, size_t size)
>  {
> -    unsigned int b;
> +    unsigned int b, len, i, c;
>  
> -    for (b = 0; b < size; b++) {
> -        if ((b % 16) == 0) {
> -            fprintf(fp, "%s: %04x:", prefix, b);
> +    for (b = 0; b < size; b += 16) {
> +        len = size - b;
> +        if (len > 16) {
> +            len = 16;
>          }
> -        if ((b % 4) == 0) {
> -            fprintf(fp, " ");
> +        fprintf(fp, "%s: %04x:", prefix, b);
> +        for (i = 0; i < 16; i++) {
> +            if ((i % 4) == 0) {
> +                fprintf(fp, " ");
> +            }
> +            if (i < len) {
> +                fprintf(fp, " %02x", (unsigned char)buf[b + i]);
> +            }
> +            else
> +            {
> +                fprintf(fp, "   ");
> +            }
>          }
> -        fprintf(fp, " %02x", (unsigned char)buf[b]);
> -        if ((b % 16) == 15) {
> -            fprintf(fp, "\n");
> +        fprintf(fp, " ");
> +        for (i = 0; i < len; i++) {
> +            c = buf[b+i];
> +            if (c < ' ' || c > '~') {
> +                c = '.';
> +            }
> +            fprintf(fp, "%c", c);
>          }
> -    }
> -    if ((b % 16) != 0) {
>          fprintf(fp, "\n");
>      }
>  }
> 

Don't forget to CC a maintainer when you submit a patch to the open
list, otherwise you run the risk of having your patch get lost. See:
http://wiki.qemu.org/Contribute/SubmitAPatch#CC_the_relevant_maintainer

CCing net/net.c maintainer Jason Wang and the contributor who suggested
the BiteSizedTask, Thomas Huth (...I think. I'm not sure who else 'huth'
on the wiki might be, but he'll certainly correct me if I'm wrong...!)

Thanks,
--js

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] util: Improved qemu_hexmap() to include an ascii dump of the buffer
  2016-03-23  0:14 ` John Snow
@ 2016-03-23  8:04   ` Thomas Huth
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Huth @ 2016-03-23  8:04 UTC (permalink / raw)
  To: John Snow, Isaac Lozano
  Cc: Peter Crosthwaite, Jason Wang, qemu-devel, Gerd Hoffmann

On 23.03.2016 01:14, John Snow wrote:
> 
> On 03/22/2016 03:41 AM, Isaac Lozano wrote:
>> qemu_hexdump() in util/hexdump.c has been changed to give also include a
>> ascii dump of the buffer. Also, calls to hex_dump() in net/net.c have
>> been replaced with calls to qemu_hexdump(). This takes care of two misc
>> BiteSized Tasks.
>>
>> Signed-off-by: Isaac Lozano <109lozanoi@gmail.com>
>> ---
>>  net/net.c      | 31 +------------------------------
>>  util/hexdump.c | 35 ++++++++++++++++++++++++-----------
>>  2 files changed, 25 insertions(+), 41 deletions(-)
>>
>> diff --git a/net/net.c b/net/net.c
>> index 1a78edf..8d51ffb 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -79,34 +79,6 @@ int default_net = 1;
>>  /***********************************************************/
>>  /* network device redirectors */
>>  
>> -#if defined(DEBUG_NET)
>> -static void hex_dump(FILE *f, const uint8_t *buf, int size)
>> -{
>> -    int len, i, j, c;
>> -
>> -    for(i=0;i<size;i+=16) {
>> -        len = size - i;
>> -        if (len > 16)
>> -            len = 16;
>> -        fprintf(f, "%08x ", i);
>> -        for(j=0;j<16;j++) {
>> -            if (j < len)
>> -                fprintf(f, " %02x", buf[i+j]);
>> -            else
>> -                fprintf(f, "   ");
>> -        }
>> -        fprintf(f, " ");
>> -        for(j=0;j<len;j++) {
>> -            c = buf[i+j];
>> -            if (c < ' ' || c > '~')
>> -                c = '.';
>> -            fprintf(f, "%c", c);
>> -        }
>> -        fprintf(f, "\n");
>> -    }
>> -}
>> -#endif
>> -
>>  static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
>>  {
>>      const char *p, *p1;
>> @@ -661,8 +633,7 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender,
>>      int ret;
>>  
>>  #ifdef DEBUG_NET
>> -    printf("qemu_send_packet_async:\n");
>> -    hex_dump(stdout, buf, size);
>> +    qemu_hexdump((const char*)buf, stdout, "qemu_send_packet_async", size);
>>  #endif
>>  
>>      if (sender->link_down || !sender->peer) {
>> diff --git a/util/hexdump.c b/util/hexdump.c
>> index 1d9c129..886e53c 100644
>> --- a/util/hexdump.c
>> +++ b/util/hexdump.c
>> @@ -18,21 +18,34 @@
>>  
>>  void qemu_hexdump(const char *buf, FILE *fp, const char *prefix, size_t size)
>>  {
>> -    unsigned int b;
>> +    unsigned int b, len, i, c;
>>  
>> -    for (b = 0; b < size; b++) {
>> -        if ((b % 16) == 0) {
>> -            fprintf(fp, "%s: %04x:", prefix, b);
>> +    for (b = 0; b < size; b += 16) {
>> +        len = size - b;
>> +        if (len > 16) {
>> +            len = 16;
>>          }
>> -        if ((b % 4) == 0) {
>> -            fprintf(fp, " ");
>> +        fprintf(fp, "%s: %04x:", prefix, b);
>> +        for (i = 0; i < 16; i++) {
>> +            if ((i % 4) == 0) {
>> +                fprintf(fp, " ");
>> +            }
>> +            if (i < len) {
>> +                fprintf(fp, " %02x", (unsigned char)buf[b + i]);
>> +            }
>> +            else
>> +            {
>> +                fprintf(fp, "   ");
>> +            }
>>          }
>> -        fprintf(fp, " %02x", (unsigned char)buf[b]);
>> -        if ((b % 16) == 15) {
>> -            fprintf(fp, "\n");
>> +        fprintf(fp, " ");
>> +        for (i = 0; i < len; i++) {
>> +            c = buf[b+i];
>> +            if (c < ' ' || c > '~') {
>> +                c = '.';
>> +            }
>> +            fprintf(fp, "%c", c);
>>          }
>> -    }
>> -    if ((b % 16) != 0) {
>>          fprintf(fp, "\n");
>>      }
>>  }
>>
> 
> Don't forget to CC a maintainer when you submit a patch to the open
> list, otherwise you run the risk of having your patch get lost. See:
> http://wiki.qemu.org/Contribute/SubmitAPatch#CC_the_relevant_maintainer
> 
> CCing net/net.c maintainer Jason Wang and the contributor who suggested
> the BiteSizedTask, Thomas Huth (...I think. I'm not sure who else 'huth'
> on the wiki might be, but he'll certainly correct me if I'm wrong...!)

Thanks, that was indeed me - and I would have indeed missed this patch
if you'd not put me on CC:. And I think you could also put Peter
Crosthwaite and Gerd Hoffmann on CC since they are mentioned as author
in the header comment of the util/hexdump.c file.

 Thomas

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] util: Improved qemu_hexmap() to include an ascii dump of the buffer
  2016-03-22  7:41 [Qemu-devel] [PATCH] util: Improved qemu_hexmap() to include an ascii dump of the buffer Isaac Lozano
  2016-03-23  0:14 ` John Snow
@ 2016-03-23  8:11 ` Thomas Huth
  1 sibling, 0 replies; 4+ messages in thread
From: Thomas Huth @ 2016-03-23  8:11 UTC (permalink / raw)
  To: Isaac Lozano, qemu-devel
  Cc: Peter Crosthwaite, Jason Wang, John Snow, Gerd Hoffmann

 Hi Isaac,

thanks for taking care of this!
Comments below...

On 22.03.2016 08:41, Isaac Lozano wrote:
> qemu_hexdump() in util/hexdump.c has been changed to give also include a
> ascii dump of the buffer. Also, calls to hex_dump() in net/net.c have
> been replaced with calls to qemu_hexdump(). This takes care of two misc
> BiteSized Tasks.
> 
> Signed-off-by: Isaac Lozano <109lozanoi@gmail.com>
> ---
>  net/net.c      | 31 +------------------------------
>  util/hexdump.c | 35 ++++++++++++++++++++++++-----------
>  2 files changed, 25 insertions(+), 41 deletions(-)
> 
> diff --git a/net/net.c b/net/net.c
> index 1a78edf..8d51ffb 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -79,34 +79,6 @@ int default_net = 1;
>  /***********************************************************/
>  /* network device redirectors */
>  
> -#if defined(DEBUG_NET)
> -static void hex_dump(FILE *f, const uint8_t *buf, int size)
> -{
> -    int len, i, j, c;
> -
> -    for(i=0;i<size;i+=16) {
> -        len = size - i;
> -        if (len > 16)
> -            len = 16;
> -        fprintf(f, "%08x ", i);
> -        for(j=0;j<16;j++) {
> -            if (j < len)
> -                fprintf(f, " %02x", buf[i+j]);
> -            else
> -                fprintf(f, "   ");
> -        }
> -        fprintf(f, " ");
> -        for(j=0;j<len;j++) {
> -            c = buf[i+j];
> -            if (c < ' ' || c > '~')
> -                c = '.';
> -            fprintf(f, "%c", c);
> -        }
> -        fprintf(f, "\n");
> -    }
> -}
> -#endif
> -
>  static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
>  {
>      const char *p, *p1;
> @@ -661,8 +633,7 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender,
>      int ret;
>  
>  #ifdef DEBUG_NET
> -    printf("qemu_send_packet_async:\n");
> -    hex_dump(stdout, buf, size);
> +    qemu_hexdump((const char*)buf, stdout, "qemu_send_packet_async", size);

Since the third parameter of qemu_hexdump() is used as a prefix for
each line, I'd maybe use a shorter string here instead, like "net",
and keep the printf("qemu_send_packet_async:\n") in front of it.

>  #endif
>  
>      if (sender->link_down || !sender->peer) {
> diff --git a/util/hexdump.c b/util/hexdump.c
> index 1d9c129..886e53c 100644
> --- a/util/hexdump.c
> +++ b/util/hexdump.c
> @@ -18,21 +18,34 @@
>  
>  void qemu_hexdump(const char *buf, FILE *fp, const char *prefix, size_t size)
>  {
> -    unsigned int b;
> +    unsigned int b, len, i, c;
>  
> -    for (b = 0; b < size; b++) {
> -        if ((b % 16) == 0) {
> -            fprintf(fp, "%s: %04x:", prefix, b);
> +    for (b = 0; b < size; b += 16) {
> +        len = size - b;
> +        if (len > 16) {
> +            len = 16;
>          }
> -        if ((b % 4) == 0) {
> -            fprintf(fp, " ");
> +        fprintf(fp, "%s: %04x:", prefix, b);
> +        for (i = 0; i < 16; i++) {
> +            if ((i % 4) == 0) {
> +                fprintf(fp, " ");
> +            }
> +            if (i < len) {
> +                fprintf(fp, " %02x", (unsigned char)buf[b + i]);
> +            }
> +            else
> +            {
> +                fprintf(fp, "   ");
> +            }
>          }
> -        fprintf(fp, " %02x", (unsigned char)buf[b]);
> -        if ((b % 16) == 15) {
> -            fprintf(fp, "\n");
> +        fprintf(fp, " ");
> +        for (i = 0; i < len; i++) {
> +            c = buf[b+i];
> +            if (c < ' ' || c > '~') {
> +                c = '.';
> +            }
> +            fprintf(fp, "%c", c);
>          }
> -    }
> -    if ((b % 16) != 0) {
>          fprintf(fp, "\n");
>      }
>  }
> 

The code looks basically fine to me, but scripts/checkpatch.pl
complains about some style issues:

ERROR: "(foo*)" should be "(foo *)"
#59: FILE: net/net.c:636:
+    qemu_hexdump((const char*)buf, stdout, "qemu_send_packet_async", size);

ERROR: that open brace { should be on the previous line
#92: FILE: util/hexdump.c:36:
+            else
+            {

ERROR: else should follow close brace '}'
#92: FILE: util/hexdump.c:36:
+            }
+            else

ERROR: spaces required around that '+' (ctx:VxV)
#102: FILE: util/hexdump.c:43:
+            c = buf[b+i];
                      ^

 Thomas

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-03-23  8:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-22  7:41 [Qemu-devel] [PATCH] util: Improved qemu_hexmap() to include an ascii dump of the buffer Isaac Lozano
2016-03-23  0:14 ` John Snow
2016-03-23  8:04   ` Thomas Huth
2016-03-23  8:11 ` Thomas Huth

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.