DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Wiles, Keith" <keith.wiles@intel.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 1/2] hexdump: whitespace cleanup
Date: Thu, 16 Aug 2018 16:34:12 +0000	[thread overview]
Message-ID: <C7B533E5-6755-4F52-BA8E-21E5022EF581@intel.com> (raw)
In-Reply-To: <20180816153106.7608-2-stephen@networkplumber.org>



> 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

  reply	other threads:[~2018-08-16 16:34 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-16 15:31 [dpdk-dev] [PATCH 0/2] eal: hexcump trivial changes Stephen Hemminger
2018-08-16 15:31 ` [dpdk-dev] [PATCH 1/2] hexdump: whitespace cleanup Stephen Hemminger
2018-08-16 16:34   ` Wiles, Keith [this message]
2018-08-16 16:59     ` Stephen Hemminger
2018-08-16 16:35   ` Wiles, Keith
2018-08-16 15:31 ` [dpdk-dev] [PATCH 2/2] hexdump: align output of string Stephen Hemminger
2018-08-16 15:50   ` Stephen Hemminger
2018-10-24 23:40 ` [dpdk-dev] [PATCH 0/2] eal: hexcump trivial changes Thomas Monjalon
2019-04-02 15:23 ` [dpdk-dev] [PATCH v2 0/2] eal: hexdump cleanups Stephen Hemminger
2019-04-02 15:23   ` Stephen Hemminger
2019-04-02 15:23   ` [dpdk-dev] [PATCH v2 1/2] hexdump: whitespace cleanup Stephen Hemminger
2019-04-02 15:23     ` Stephen Hemminger
2019-04-02 15:23   ` [dpdk-dev] [PATCH v2 2/2] hexdump: align output of string Stephen Hemminger
2019-04-02 15:23     ` Stephen Hemminger
2019-04-03 16:36   ` [dpdk-dev] [PATCH v2 0/2] eal: hexdump cleanups Thomas Monjalon
2019-04-03 16:36     ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=C7B533E5-6755-4F52-BA8E-21E5022EF581@intel.com \
    --to=keith.wiles@intel.com \
    --cc=dev@dpdk.org \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).