[PATCH] [PATCH 1 of 4] Core: add function to decode hexadecimal strings

[Maxim Dounin]

Hello!

On Wed, Aug 23, 2017 at 09:19:39PM -0500, Nate Karstens wrote:

> # HG changeset patch
> # User Nate Karstens <nate.karstens at garmin.com>
> # Date 1503540018 18000
> #      Wed Aug 23 21:00:18 2017 -0500
> # Node ID 17c038b56051f922ec440d40e23e8d1b23d835df
> # Parent  c7d4017c8876af6d8570e400320537d7d39e9578
> [PATCH 1 of 4] Core: add function to decode hexadecimal strings.

There is no need to include "[PATCH 1 of 4] " into the patch 
summary line.

> 
> Adds functionality to convert a hexadecimal string into binary data.
> This will be used to decode PSKs stored in hexadecimal representation.
> 
> Signed-off-by: Nate Karstens <nate.karstens at garmin.com>
> 
> diff -r c7d4017c8876 -r 17c038b56051 src/core/ngx_string.c
> --- a/src/core/ngx_string.c     Tue Aug 22 21:22:59 2017 +0300
> +++ b/src/core/ngx_string.c     Wed Aug 23 21:00:18 2017 -0500
> @@ -1118,6 +1118,57 @@ ngx_hex_dump(u_char *dst, u_char *src, s
>  }
> 
> 
> +ngx_int_t
> +ngx_hex_decode(u_char *dst, u_char *src, size_t len)
> +{
> +    u_char     ch, decoded;

Style: there should be two spaces between the (longest) type and 
variables.

> +
> +    if (len & 1) {
> +        return NGX_ERROR;
> +    }
> +
> +    while (len > 0) {
> +        ch = *src++;
> +        len--;

It might worth rewriting this to be more in line with 
ngx_hex_dump() as

    while (len--) {
        ch = *src++;

> +
> +        if (len & 1) {
> +
> +            if (ch >= '0' && ch <= '9') {
> +                decoded = ch - '0';
> +                continue;
> +            }
> +
> +            ch |= 0x20;
> +
> +            if (ch >= 'a' && ch <= 'f') {
> +                decoded = ch - 'a' + 10;
> +                continue;
> +            }
> +
> +            return NGX_ERROR;
> +
> +        } else {
> +
> +            if (ch >= '0' && ch <= '9') {
> +                *dst++ = (u_char) ((decoded << 4) + ch - '0');

Here compilation with at least Clang fails with the following error:

src/core/ngx_string.c:1153:37: error: variable 'decoded' may be uninitialized
      when used here [-Werror,-Wconditional-uninitialized]
                *dst++ = (u_char) ((decoded << 4) + ch - '0');
                                    ^~~~~~~

This looks like false positive, though needs fixing anyway.  For 
me, most trivial solution would be to unroll the loop instead of 
relying on the non-trivial (len & 1) check.

[...]

-- 
Maxim Dounin
http://nginx.org/