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

["Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>]

> -----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