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

[Dumitrescu, Cristian]

> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Monday, October 19, 2015 4:50 PM
> To: Singh, Jasvinder <jasvinder.singh at intel.com>
> Cc: dev at 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 at 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