Re: [dpdk-dev] [PATCH 1/2] hexdump: whitespace cleanup

["Wiles, Keith" <keith.wiles@intel.com>]

> 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