[dpdk-dev,v2,6/8] example/ip_pipeline: add parse_hex_string for internal use

Message ID 1444744652-573-7-git-send-email-jasvinder.singh@intel.com (mailing list archive)
State Changes Requested, archived
Headers

Commit Message

Jasvinder Singh Oct. 13, 2015, 1:57 p.m. UTC
  From: Fan Zhang <roy.fan.zhang@intel.com>

This patch adds parse_hex_string function to parse hex string to uint8_t
array.

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
---
 examples/ip_pipeline/config_parse.c | 70 +++++++++++++++++++++++++++++++++++++
 examples/ip_pipeline/pipeline.h     |  4 +++
 2 files changed, 74 insertions(+)
  

Comments

Thomas Monjalon Oct. 19, 2015, 1:49 p.m. UTC | #1
2015-10-13 14:57, Jasvinder Singh:
> From: Fan Zhang <roy.fan.zhang@intel.com>
> +static uint32_t
> +get_hex_val(char c)
> +{
> +	switch (c) {
> +	case '0':
> +	case '1':
> +	case '2':
> +	case '3':
> +	case '4':
> +	case '5':
> +	case '6':
> +	case '7':
> +	case '8':
> +	case '9':
> +		return c - '0';
> +	case 'A':
> +	case 'B':
> +	case 'C':
> +	case 'D':
> +	case 'E':
> +	case 'F':
> +		return c - 'A' + 10;
> +	case 'a':
> +	case 'b':
> +	case 'c':
> +	case 'd':
> +	case 'e':
> +	case 'f':
> +		return c - 'a' + 10;
> +	default:
> +		return 0;
> +	}
> +}
> +
> +int
> +parse_hex_string(char *src, uint8_t *dst, uint32_t *size)
> +{
> +	char *c;
> +	uint32_t len, i;
> +
> +	/* Check input parameters */
> +	if ((src == NULL) ||
> +		(dst == NULL) ||
> +		(size == NULL) ||
> +		(*size == 0))
> +		return -1;
> +
> +	len = strlen(src);
> +	if (((len & 3) != 0) ||
> +		(len > (*size) * 2))
> +		return -1;
> +	*size = len / 2;
> +
> +	for (c = src; *c != 0; c++) {
> +		if ((((*c) >= '0') && ((*c) <= '9')) ||
> +			(((*c) >= 'A') && ((*c) <= 'F')) ||
> +			(((*c) >= 'a') && ((*c) <= 'f')))
> +			continue;
> +
> +		return -1;
> +	}
> +
> +	/* Convert chars to bytes */
> +	for (i = 0; i < *size; i++)
> +		dst[i] = get_hex_val(src[2 * i]) * 16 +
> +			get_hex_val(src[2 * i + 1]);
> +
> +	return 0;
> +}

Why not use strtol()?
  
Cristian Dumitrescu Oct. 20, 2015, 2:31 p.m. UTC | #2
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Monday, October 19, 2015 4:50 PM
> To: Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 6/8] example/ip_pipeline: add
> parse_hex_string for internal use
> 
> 2015-10-13 14:57, Jasvinder Singh:
> > From: Fan Zhang <roy.fan.zhang@intel.com>
> > +static uint32_t
> > +get_hex_val(char c)
> > +{
> > +	switch (c) {
> > +	case '0':
> > +	case '1':
> > +	case '2':
> > +	case '3':
> > +	case '4':
> > +	case '5':
> > +	case '6':
> > +	case '7':
> > +	case '8':
> > +	case '9':
> > +		return c - '0';
> > +	case 'A':
> > +	case 'B':
> > +	case 'C':
> > +	case 'D':
> > +	case 'E':
> > +	case 'F':
> > +		return c - 'A' + 10;
> > +	case 'a':
> > +	case 'b':
> > +	case 'c':
> > +	case 'd':
> > +	case 'e':
> > +	case 'f':
> > +		return c - 'a' + 10;
> > +	default:
> > +		return 0;
> > +	}
> > +}
> > +
> > +int
> > +parse_hex_string(char *src, uint8_t *dst, uint32_t *size)
> > +{
> > +	char *c;
> > +	uint32_t len, i;
> > +
> > +	/* Check input parameters */
> > +	if ((src == NULL) ||
> > +		(dst == NULL) ||
> > +		(size == NULL) ||
> > +		(*size == 0))
> > +		return -1;
> > +
> > +	len = strlen(src);
> > +	if (((len & 3) != 0) ||
> > +		(len > (*size) * 2))
> > +		return -1;
> > +	*size = len / 2;
> > +
> > +	for (c = src; *c != 0; c++) {
> > +		if ((((*c) >= '0') && ((*c) <= '9')) ||
> > +			(((*c) >= 'A') && ((*c) <= 'F')) ||
> > +			(((*c) >= 'a') && ((*c) <= 'f')))
> > +			continue;
> > +
> > +		return -1;
> > +	}
> > +
> > +	/* Convert chars to bytes */
> > +	for (i = 0; i < *size; i++)
> > +		dst[i] = get_hex_val(src[2 * i]) * 16 +
> > +			get_hex_val(src[2 * i + 1]);
> > +
> > +	return 0;
> > +}
> 
> Why not use strtol()?

Strtol() was considered, but the code would have been more complicated (more corner case errors to handle) and less readable (more difficult to maintain), so our vote went for this simpler solution.

This function needs to convert an array of characters of up to 128 hex digits to the associated array of bytes, with output byte on position I representing the value of the i-th pair of input characters. For example, "010203" gets converted to array [1, 2, 3]. It only needs very simple math, which leads to the above code, which looks very readable.

Strtol() was not really intended for this. Strtol() is primarily intended to convert up to 8 characters to the associated value. Using strtol() would be possible to use (to convert each 2 characters to the byte) but we end up with a lot of error cases to handle, which leads to more complex code in our opinion.

Thanks,
Cristian
  

Patch

diff --git a/examples/ip_pipeline/config_parse.c b/examples/ip_pipeline/config_parse.c
index c9b78f9..d7ee707 100644
--- a/examples/ip_pipeline/config_parse.c
+++ b/examples/ip_pipeline/config_parse.c
@@ -455,6 +455,76 @@  parse_pipeline_core(uint32_t *socket,
 	return 0;
 }
 
+static uint32_t
+get_hex_val(char c)
+{
+	switch (c) {
+	case '0':
+	case '1':
+	case '2':
+	case '3':
+	case '4':
+	case '5':
+	case '6':
+	case '7':
+	case '8':
+	case '9':
+		return c - '0';
+	case 'A':
+	case 'B':
+	case 'C':
+	case 'D':
+	case 'E':
+	case 'F':
+		return c - 'A' + 10;
+	case 'a':
+	case 'b':
+	case 'c':
+	case 'd':
+	case 'e':
+	case 'f':
+		return c - 'a' + 10;
+	default:
+		return 0;
+	}
+}
+
+int
+parse_hex_string(char *src, uint8_t *dst, uint32_t *size)
+{
+	char *c;
+	uint32_t len, i;
+
+	/* Check input parameters */
+	if ((src == NULL) ||
+		(dst == NULL) ||
+		(size == NULL) ||
+		(*size == 0))
+		return -1;
+
+	len = strlen(src);
+	if (((len & 3) != 0) ||
+		(len > (*size) * 2))
+		return -1;
+	*size = len / 2;
+
+	for (c = src; *c != 0; c++) {
+		if ((((*c) >= '0') && ((*c) <= '9')) ||
+			(((*c) >= 'A') && ((*c) <= 'F')) ||
+			(((*c) >= 'a') && ((*c) <= 'f')))
+			continue;
+
+		return -1;
+	}
+
+	/* Convert chars to bytes */
+	for (i = 0; i < *size; i++)
+		dst[i] = get_hex_val(src[2 * i]) * 16 +
+			get_hex_val(src[2 * i + 1]);
+
+	return 0;
+}
+
 static size_t
 skip_digits(const char *src)
 {
diff --git a/examples/ip_pipeline/pipeline.h b/examples/ip_pipeline/pipeline.h
index b9a56ea..4063594 100644
--- a/examples/ip_pipeline/pipeline.h
+++ b/examples/ip_pipeline/pipeline.h
@@ -84,4 +84,8 @@  pipeline_type_cmds_count(struct pipeline_type *ptype)
 	return n_cmds;
 }
 
+/* Parse hex string to uint8_t array */
+int
+parse_hex_string(char *src, uint8_t *dst, uint32_t *size);
+
 #endif