From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 6581126D6B for ; Mon, 13 Aug 2018 10:46:41 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id xy3qtlp2mEZZ for ; Mon, 13 Aug 2018 10:46:41 -0400 (EDT) Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [94.100.177.112]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id D4AA726A3B for ; Mon, 13 Aug 2018 10:46:40 -0400 (EDT) Subject: [tarantool-patches] Re: Re[2]: Fwd: Re[2]: [PATCH] lua: add string.fromhex method References: <1533839716.912611685@f103.i.mail.ru> <1533841323.686635839@f496.i.mail.ru> From: Vladislav Shpilevoy Message-ID: <990898e3-2b5d-6fcc-c2ce-4d348a9f6a2a@tarantool.org> Date: Mon, 13 Aug 2018 17:46:37 +0300 MIME-Version: 1.0 In-Reply-To: <1533841323.686635839@f496.i.mail.ru> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org, Nikita Tatunov Hi! Thanks for the patch! See 3 comments below. > commit c208a8173b2f7632c3432d03807490becfe7276b > Author: N.Tatunov > 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)