Hello! On Fri, Jul 28, 2017 at 01:45:36PM -0500, Nate Karstens wrote:
> # HG changeset patch > # User Nate Karstens <[email protected]> > # Date 1501265787 18000 > # Fri Jul 28 13:16:27 2017 -0500 > # Node ID 7e10e1dc1fae0f538a0a74fd6783f9b33a905933 > # Parent 72d3aefc2993a639cc8cddc94aefc7c4390ee252 > Core: add function to decode hexadecimal strings. > > 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 <[email protected]> > > diff -r 72d3aefc2993 -r 7e10e1dc1fae src/core/ngx_string.c > --- a/src/core/ngx_string.c Wed Jul 26 13:13:51 2017 +0300 > +++ b/src/core/ngx_string.c Fri Jul 28 13:16:27 2017 -0500 > @@ -1104,6 +1104,54 @@ ngx_hextoi(u_char *line, size_t n) > } > > > +ngx_int_t > +ngx_hex_decode(u_char *dst, size_t dlen, u_char *src, size_t slen) > +{ > + u_char c, ch; > + ngx_int_t len; > + > + if ((slen & 1) || (dlen < (slen / 2))) { > + return NGX_ERROR; It doesn't look like dlen is useful in the interface, at least in the for it is implemented. It could make sense if the function would written in a way which allows decoding only some bytes of the source string, but a simple "buffer must be at least as large as half of the source string" prerequisite certainly doesn't need an additional parameter. For example, ngx_hex_dump() doesn't have it. > + } > + > + len = slen / 2; Returning a length known in advice might not be a good idea either. It is already known to the caller. Rather, I would recommend returning NGX_OK as, for example, ngx_decode_base64() does. > + > + while (slen > 0) { > + ch = *src; > + c = (u_char) (ch | 0x20); > + > + if (ch >= '0' && ch <= '9') { > + *dst = ch - '0'; > + } else if (c >= 'a' && c <= 'f') { > + *dst = c - 'a' + 10; It might be a good idea to introduce a u_char variable on stack, similar to how it is done in ngx_http_parse_complex_uri(). It also might be a good idea to restructure the code to avoid meaningless lowercase operations for 0..9 characters. And I see no reason to use two variables, "ch" and "c". It might also need some additional empty lines to better match style, though the code is anyway needs to be rewritten. > + } else { > + return NGX_ERROR; > + } > + > + *dst <<= 4; > + src++; It might be more logical to combine src++ with reading the character, ch = *src++; Such approach can be seen in many other functions in ngx_string.c. > + > + ch = *src; > + c = (u_char) (ch | 0x20); > + > + if (ch >= '0' && ch <= '9') { > + *dst |= ch - '0'; > + } else if (c >= 'a' && c <= 'f') { > + *dst |= c - 'a' + 10; > + } else { > + return NGX_ERROR; > + } > + > + dst++; > + src++; > + > + slen -= 2; > + } > + > + return len; > +} > + > + > u_char * > ngx_hex_dump(u_char *dst, u_char *src, size_t len) > { > diff -r 72d3aefc2993 -r 7e10e1dc1fae src/core/ngx_string.h > --- a/src/core/ngx_string.h Wed Jul 26 13:13:51 2017 +0300 > +++ b/src/core/ngx_string.h Fri Jul 28 13:16:27 2017 -0500 > @@ -176,6 +176,7 @@ off_t ngx_atoof(u_char *line, size_t n); > time_t ngx_atotm(u_char *line, size_t n); > ngx_int_t ngx_hextoi(u_char *line, size_t n); > > +ngx_int_t ngx_hex_decode(u_char *dst, size_t dlen, u_char *src, size_t slen); > u_char *ngx_hex_dump(u_char *dst, u_char *src, size_t len); It might be a good idea to change the order, so the decode function is placed after an encode one, as in other similar functions in the same files. -- Maxim Dounin http://nginx.org/ _______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
