diff mbox series

[1/2] hexdump: whitespace cleanup

Message ID 20180816153106.7608-2-stephen@networkplumber.org (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series eal: hexcump trivial changes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Stephen Hemminger Aug. 16, 2018, 3:31 p.m. UTC 
  The hexdump code obviously came from somewhere else originally.
It is not formatted according to DPDK coding style.

Also, drop the comment which is not useful the docbock comment
is already in the rte_hexdump.h

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/common/eal_common_hexdump.c | 109 ++++++++-------------
 1 file changed, 43 insertions(+), 66 deletions(-)

Comments

Wiles, Keith Aug. 16, 2018, 4:34 p.m. UTC | #1 
> On Aug 16, 2018, at 10:31 AM, Stephen Hemminger <stephen@networkplumber.org> wrote:
> 
> The hexdump code obviously came from somewhere else originally.
> It is not formatted according to DPDK coding style.
> 
> Also, drop the comment which is not useful the docbock comment
> is already in the rte_hexdump.h
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> lib/librte_eal/common/eal_common_hexdump.c | 109 ++++++++-------------
> 1 file changed, 43 insertions(+), 66 deletions(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_hexdump.c b/lib/librte_eal/common/eal_common_hexdump.c
> index 9ca7c511c062..980cf73ac337 100644
> --- a/lib/librte_eal/common/eal_common_hexdump.c
> +++ b/lib/librte_eal/common/eal_common_hexdump.c
> @@ -10,82 +10,59 @@
> 
> #define LINE_LEN 128
> 
> -/**************************************************************************//**
> -*
> -* rte_hexdump - Dump out memory in a special hex dump format.
> -*
> -* DESCRIPTION
> -* Dump out the message buffer in a special hex dump output format with characters
> -* printed for each line of 16 hex values.
> -*
> -* RETURNS: N/A
> -*
> -* SEE ALSO:
> -*/
> -
> -void
> -rte_hexdump(FILE *f, const char * title, const void * buf, unsigned int len)
> +void rte_hexdump(FILE *f, const char *title, const void *buf, unsigned int len)

I looked at the Coding Guide in section 1.7.2 and it states

1.7.2. Definitions
	- The function type should be on a line by itself preceding the function.
	- The opening brace of the function body should be on a line by itself.

	static char *
	function(int a1, int a2, float fl, int a4)
	{

I have noticed some places are not following that rule in other parts of DPDK. Function prototypes are on the same line.

> {
> -    unsigned int i, out, ofs;
> -    const unsigned char *data = buf;
> -    char line[LINE_LEN];    /* space needed 8+16*3+3+16 == 75 */
> +	unsigned int i, out, ofs;
> +	const unsigned char *data = buf;
> +	char line[LINE_LEN];	/* space needed 8+16*3+3+16 == 75 */
> 
> -    fprintf(f, "%s at [%p], len=%u\n", (title)? title  : "  Dump data", data, len);
> -    ofs = 0;
> -    while (ofs < len) {
> -        /* format the line in the buffer, then use printf to output to screen */
> -        out = snprintf(line, LINE_LEN, "%08X:", ofs);
> -        for (i = 0; ((ofs + i) < len) && (i < 16); i++)
> -            out += snprintf(line+out, LINE_LEN - out, " %02X", (data[ofs+i] & 0xff));
> -        for(; i <= 16; i++)
> -            out += snprintf(line+out, LINE_LEN - out, " | ");
> -        for(i = 0; (ofs < len) && (i < 16); i++, ofs++) {
> -            unsigned char c = data[ofs];
> -            if ( (c < ' ') || (c > '~'))
> -                c = '.';
> -            out += snprintf(line+out, LINE_LEN - out, "%c", c);
> -        }
> -        fprintf(f, "%s\n", line);
> -    }
> -    fflush(f);
> -}
> +	fprintf(f, "%s at [%p], len=%u\n", (title) ? title : "  Dump data",
> +		data, len);
> +	ofs = 0;
> +	while (ofs < len) {
> +		/* format the line in the buffer */
> +		out = snprintf(line, LINE_LEN, "%08X:", ofs);
> +		for (i = 0; i < 16 && ofs + i < len; i++)
> +			out += snprintf(line + out, LINE_LEN - out,
> +					 " %02X", (data[ofs + i] & 0xff));
> +		for (; i <= 16; i++)
> +			out += snprintf(line + out, LINE_LEN - out, " | ");
> +
> +		for (i = 0; (ofs < len) && (i < 16); i++, ofs++) {
> +			unsigned char c = data[ofs];
> 
> -/**************************************************************************//**
> -*
> -* rte_memdump - Dump out memory in hex bytes with colons.
> -*
> -* DESCRIPTION
> -* Dump out the message buffer in hex bytes with colons xx:xx:xx:xx:...
> -*
> -* RETURNS: N/A
> -*
> -* SEE ALSO:
> -*/
> +			if (c < ' ' || c > '~')
> +				c = '.';
> +			out += snprintf(line + out, LINE_LEN - out, "%c", c);
> +		}
> +		fprintf(f, "%s\n", line);
> +	}
> +	fflush(f);
> +}
> 
> -void
> -rte_memdump(FILE *f, const char * title, const void * buf, unsigned int len)
> +void rte_memdump(FILE *f, const char *title, const void *buf, unsigned int len)
> {
> -    unsigned int i, out;
> -    const unsigned char *data = buf;
> -    char line[LINE_LEN];
> +	unsigned int i, out;
> +	const unsigned char *data = buf;
> +	char line[LINE_LEN];
> 
> -    if ( title )
> -	fprintf(f, "%s: ", title);
> +	if (title)
> +		fprintf(f, "%s: ", title);
> 
> -    line[0] = '\0';
> -    for (i = 0, out = 0; i < len; i++) {
> -	// Make sure we do not overrun the line buffer length.
> -		if ( out >= (LINE_LEN - 4) ) {
> +	line[0] = '\0';
> +	for (i = 0, out = 0; i < len; i++) {
> +		/* Make sure we do not overrun the line buffer length. */
> +		if (out >= (LINE_LEN - 4)) {
> 			fprintf(f, "%s", line);
> 			out = 0;
> 			line[out] = '\0';
> 		}
> -		out += snprintf(line+out, LINE_LEN - out, "%02x%s",
> -				(data[i] & 0xff), ((i+1) < len)? ":" : "");
> -    }
> -    if ( out > 0 )
> -	fprintf(f, "%s", line);
> -    fprintf(f, "\n");
> +		out += snprintf(line + out, LINE_LEN - out, "%02x%s",
> +				(data[i] & 0xff), ((i + 1) < len) ? ":" : "");
> +	}
> +	if (out > 0)
> +		fprintf(f, "%s", line);
> +	fprintf(f, "\n");
> 
> -    fflush(f);
> +	fflush(f);
> }
> -- 
> 2.18.0
> 

Regards,
Keith
Wiles, Keith Aug. 16, 2018, 4:35 p.m. UTC | #2 
> On Aug 16, 2018, at 10:31 AM, Stephen Hemminger <stephen@networkplumber.org> wrote:
> 
> The hexdump code obviously came from somewhere else originally.
> It is not formatted according to DPDK coding style.
> 
> Also, drop the comment which is not useful the docbock comment
> is already in the rte_hexdump.h
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> —

This code was put in years ago, maybe before it was push to public repo :-)
Thanks for the cleanup and output cleanup.

Regards,
Keith
Stephen Hemminger Aug. 16, 2018, 4:59 p.m. UTC | #3 
On Thu, 16 Aug 2018 16:34:12 +0000
"Wiles, Keith" <keith.wiles@intel.com> wrote:

> I looked at the Coding Guide in section 1.7.2 and it states
> 
> 1.7.2. Definitions
> 	- The function type should be on a line by itself preceding the function.
> 	- The opening brace of the function body should be on a line by itself.
> 
> 	static char *
> 	function(int a1, int a2, float fl, int a4)
> 	{


Really, I thought in general DPDK followed Linux kernel style.
Making little exceptions like this is a nuisance.
diff mbox series

Patch

diff --git a/lib/librte_eal/common/eal_common_hexdump.c b/lib/librte_eal/common/eal_common_hexdump.c
index 9ca7c511c062..980cf73ac337 100644
--- a/lib/librte_eal/common/eal_common_hexdump.c
+++ b/lib/librte_eal/common/eal_common_hexdump.c
@@ -10,82 +10,59 @@ 
 
 #define LINE_LEN 128
 
-/**************************************************************************//**
-*
-* rte_hexdump - Dump out memory in a special hex dump format.
-*
-* DESCRIPTION
-* Dump out the message buffer in a special hex dump output format with characters
-* printed for each line of 16 hex values.
-*
-* RETURNS: N/A
-*
-* SEE ALSO:
-*/
-
-void
-rte_hexdump(FILE *f, const char * title, const void * buf, unsigned int len)
+void rte_hexdump(FILE *f, const char *title, const void *buf, unsigned int len)
 {
-    unsigned int i, out, ofs;
-    const unsigned char *data = buf;
-    char line[LINE_LEN];    /* space needed 8+16*3+3+16 == 75 */
+	unsigned int i, out, ofs;
+	const unsigned char *data = buf;
+	char line[LINE_LEN];	/* space needed 8+16*3+3+16 == 75 */
 
-    fprintf(f, "%s at [%p], len=%u\n", (title)? title  : "  Dump data", data, len);
-    ofs = 0;
-    while (ofs < len) {
-        /* format the line in the buffer, then use printf to output to screen */
-        out = snprintf(line, LINE_LEN, "%08X:", ofs);
-        for (i = 0; ((ofs + i) < len) && (i < 16); i++)
-            out += snprintf(line+out, LINE_LEN - out, " %02X", (data[ofs+i] & 0xff));
-        for(; i <= 16; i++)
-            out += snprintf(line+out, LINE_LEN - out, " | ");
-        for(i = 0; (ofs < len) && (i < 16); i++, ofs++) {
-            unsigned char c = data[ofs];
-            if ( (c < ' ') || (c > '~'))
-                c = '.';
-            out += snprintf(line+out, LINE_LEN - out, "%c", c);
-        }
-        fprintf(f, "%s\n", line);
-    }
-    fflush(f);
-}
+	fprintf(f, "%s at [%p], len=%u\n", (title) ? title : "  Dump data",
+		data, len);
+	ofs = 0;
+	while (ofs < len) {
+		/* format the line in the buffer */
+		out = snprintf(line, LINE_LEN, "%08X:", ofs);
+		for (i = 0; i < 16 && ofs + i < len; i++)
+			out += snprintf(line + out, LINE_LEN - out,
+					 " %02X", (data[ofs + i] & 0xff));
+		for (; i <= 16; i++)
+			out += snprintf(line + out, LINE_LEN - out, " | ");
+
+		for (i = 0; (ofs < len) && (i < 16); i++, ofs++) {
+			unsigned char c = data[ofs];
 
-/**************************************************************************//**
-*
-* rte_memdump - Dump out memory in hex bytes with colons.
-*
-* DESCRIPTION
-* Dump out the message buffer in hex bytes with colons xx:xx:xx:xx:...
-*
-* RETURNS: N/A
-*
-* SEE ALSO:
-*/
+			if (c < ' ' || c > '~')
+				c = '.';
+			out += snprintf(line + out, LINE_LEN - out, "%c", c);
+		}
+		fprintf(f, "%s\n", line);
+	}
+	fflush(f);
+}
 
-void
-rte_memdump(FILE *f, const char * title, const void * buf, unsigned int len)
+void rte_memdump(FILE *f, const char *title, const void *buf, unsigned int len)
 {
-    unsigned int i, out;
-    const unsigned char *data = buf;
-    char line[LINE_LEN];
+	unsigned int i, out;
+	const unsigned char *data = buf;
+	char line[LINE_LEN];
 
-    if ( title )
-	fprintf(f, "%s: ", title);
+	if (title)
+		fprintf(f, "%s: ", title);
 
-    line[0] = '\0';
-    for (i = 0, out = 0; i < len; i++) {
-	// Make sure we do not overrun the line buffer length.
-		if ( out >= (LINE_LEN - 4) ) {
+	line[0] = '\0';
+	for (i = 0, out = 0; i < len; i++) {
+		/* Make sure we do not overrun the line buffer length. */
+		if (out >= (LINE_LEN - 4)) {
 			fprintf(f, "%s", line);
 			out = 0;
 			line[out] = '\0';
 		}
-		out += snprintf(line+out, LINE_LEN - out, "%02x%s",
-				(data[i] & 0xff), ((i+1) < len)? ":" : "");
-    }
-    if ( out > 0 )
-	fprintf(f, "%s", line);
-    fprintf(f, "\n");
+		out += snprintf(line + out, LINE_LEN - out, "%02x%s",
+				(data[i] & 0xff), ((i + 1) < len) ? ":" : "");
+	}
+	if (out > 0)
+		fprintf(f, "%s", line);
+	fprintf(f, "\n");
 
-    fflush(f);
+	fflush(f);
 }