| Message ID | 1458632497-27173-1-git-send-email-109lozanoi@gmail.com |
|---|---|
| State | New |
| Headers | show |
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
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
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
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"); } }
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(-)