Hi! Thanks for the patch!
See 3 comments below.
> commit c208a8173b2f7632c3432d03807490becfe7276b
> Author: N.Tatunov <n.tatunov@tarantool.org>
> Date: Wed Aug 8 14:17:02 2018 +0300
>
> lua: add string.fromhex method
>
> Add string.fromhex method. Add test for string.fromhex().
>
> Closes #2562
>
> diff --git a/src/lua/string.lua b/src/lua/string.lua
> index 5ff64c9f6..f6accb42c 100644
> --- a/src/lua/string.lua
> +++ b/src/lua/string.lua
> @@ -292,6 +292,54 @@ local function string_hex(inp)
> return ffi.string(res, len)
> end
>
> +local hexadecimal_chars = {
> + '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A',
> + 'a', 'B', 'b', 'C', 'c', 'D', 'd', 'E', 'e', 'F', 'f'}
> +
> +local hexadecimal_values = {
> + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10,
> + 10, 11, 11, 12, 12, 13, 13, 14, 14, 15, 15}
> +
> +local hexadecimals_mapping = {}
> +
> +local function chars_to_hex(chars, values, mapping)
> + for i, char in ipairs(chars) do
> + mapping[string.byte(char)] = values[i]
> + end
> +end
1. Why do you need this function to be called only once 1 line
below? Can you just inline it?
> +
> +chars_to_hex(hexadecimal_chars, hexadecimal_values, hexadecimals_mapping)
> +
> +--- Match a hexadecimal representation of a string to its
> +-- string representation.
> +-- @function fromhex
> +-- @string inp the string of hexadecimals
> +-- @returns formatted string
2. I know that such comments style you got from other functions
of the file, but each line above contradicts with our official
code style, as I understand. Just for record.
> +local function string_fromhex(inp)
> + if type(inp) ~= 'string' then
> + error(err_string_arg:format(1, 'string.fromhex', 'string',
> + type(inp)), 2)
> + end
> + if inp:len() % 2 ~= 0 then
> + error(err_string_arg:format(1, 'string.fromhex',
> + 'even amount of chars',
> + 'odd amount'), 2)
> + end
> + local len = inp:len() / 2
> + local casted_inp = ffi.cast('const char *', inp)
> + local res = ffi.new('char[?]', len)
3. Why '?'? You know that res will be exactly len bytes.
> + for i = 0, len - 1 do
> + local first = hexadecimals_mapping[casted_inp[i * 2]]
> + local second = hexadecimals_mapping[casted_inp[i * 2 + 1]]
> + if first == nil or second == nil then
> + error(err_string_arg:format(1, 'string.fromhex', 'hex string',
> + 'non hex chars'), 2)
> + end
> + res[i] = first * 16 + second
> + end
> + return ffi.string(res, len)
> +end
> +
> local function string_strip(inp)
> if type(inp) ~= 'string' then
> error(err_string_arg:format(1, "string.strip", 'string', type(inp)), 2)