<HTML><BODY><div class="js-helper js-readmsg-msg">
<style type="text/css"></style>
<div>
<base target="_self" href="https://e.mail.ru/">
<div id="style_15794967510907455334_BODY"><div class="class_1582823965">
<p>Igor,<br><br>Thanks for the review, I've made all the changes as you suggested.<br><br></p><blockquote style="border-left:1px solid #0857A6;margin:10px;padding:0 0 0 10px;">
Понедельник, 13 января 2020, 15:06 +03:00 от Igor Munkin <<a href="mailto:imun@tarantool.org">imun@tarantool.org</a>>:<br>
<br>
<div id="">
<div class="js-helper_mailru_css_attribute_postfix js-readmsg-msg_mailru_css_attribute_postfix">
<style></style>
<div>
<div id="style_15789172191310020357_BODY_mailru_css_attribute_postfix">Sasha,<br>
<br>
Thanks for the patch! I left several comments below, please consider<br>
them.<br>
<br>
On 16.12.19, Alexander V. Tikhonov wrote:<br>
> Added local definitions for the variables, added os.exit()<br>
> routine at the tests finish. Changed added information messages<br>
> to test:ok() calls.<br>
<br>
IMHO bullet list is a way more readable for listing the changes.<br>
<br>
> <br>
> Part of #4655<br>
> ---<br>
> test/fix_string_find_recording.test.lua | 6 +++---<br>
> test/fold_bug_LuaJIT_505.test.lua | 7 ++++---<br>
> test/fold_bug_LuaJIT_524.test.lua | 2 +-<br>
> test/gh.test.lua | 8 ++++----<br>
> test/pairsmm_tarantool_4560.test.lua | 8 +++++---<br>
> test/table_chain_bug_LuaJIT_494.test.lua | 23 ++++++++++++-----------<br>
> test/unsink_64_kptr.test.lua | 7 ++++---<br>
> 7 files changed, 33 insertions(+), 28 deletions(-)<br>
<br>
General comments:<br>
* Feel free to enhance the existing test names passed to tap.test:<br>
| $ grep -rF 'tap.test'<br>
| table_chain_bug_LuaJIT_494.test.lua:local test = tap.test("494")<br>
| fix_string_find_recording.test.lua:local test =<br>
| tap.test("fix-string-find-recording")<br>
| gh.test.lua:local test = tap.test("gh")<br>
| pairsmm_tarantool_4560.test.lua:local test = tap.test("PAIRSMM-is-set")<br>
| fold_bug_LuaJIT_505.test.lua:local test = tap.test("505")<br>
| fold_bug_LuaJIT_524.test.lua:local test = tap.test("LuaJIT 524")<br>
| unsink_64_kptr.test.lua:local test = tap.test("232")<br>
* It's a good idea to add more verbose description, but the new ones<br>
still don't make sense to me:<br>
| $ grep -rnF 'test:ok'<br>
| table_chain_bug_LuaJIT_494.test.lua:177:test:ok(true, "LuaJIT/LuaJIT gh-494 successfully checked commit a7dc9e8d23315217c9fe0029bc8ae12c03306b33")<br>
| fold_bug_LuaJIT_505.test.lua:19:test:ok(true, "LuaJIT/LuaJIT gh-505 assert not occured")<br>
| unsink_64_kptr.test.lua:43:test:ok(true, "openresty-lua/resty-core gh-232 allowed its address to be inlined")<br>
<br>
> <br>
> diff --git a/test/fix_string_find_recording.test.lua b/test/fix_string_find_recording.test.lua<br>
> index d3fc9e1..911e611 100755<br>
> --- a/test/fix_string_find_recording.test.lua<br>
> +++ b/test/fix_string_find_recording.test.lua<br>
> @@ -1,8 +1,8 @@<br>
> #!/usr/bin/env tarantool<br>
> <br>
> -tap = require('tap')<br>
> +local tap = require('tap')<br>
> <br>
> -test = tap.test("fix-string-find-recording")<br>
> +local test = tap.test("fix-string-find-recording")<br>
> test:plan(1)<br>
> <br>
> local err = [[module 'kit.1.10.3-136' not found:<br>
> @@ -76,4 +76,4 @@ until not e<br>
> <br>
> test:is(count_vm, count_jit)<br>
> <br>
> -test:check()<br>
> +os.exit(test:check() and 0 or 1)<br>
> diff --git a/test/fold_bug_LuaJIT_505.test.lua b/test/fold_bug_LuaJIT_505.test.lua<br>
> index 2fee069..433efb0 100755<br>
> --- a/test/fold_bug_LuaJIT_505.test.lua<br>
> +++ b/test/fold_bug_LuaJIT_505.test.lua<br>
> @@ -1,8 +1,8 @@<br>
> #!/usr/bin/env tarantool<br>
> <br>
> -tap = require('tap')<br>
> +local tap = require('tap')<br>
> <br>
> -test = tap.test("505")<br>
> +local test = tap.test("505")<br>
> test:plan(1)<br>
> <br>
> -- Test file to demonstrate Lua fold machinery icorrect behavior, details:<br>
> @@ -16,5 +16,6 @@ for _ = 1, 20 do<br>
> local pos_b = string.find(value2, "b", 2, true)<br>
> assert(pos_b == 2, "FAIL: position of 'b' is " .. pos_b)<br>
> end<br>
> +test:ok(true, "LuaJIT/LuaJIT gh-505 assert not occured")<br>
> <br>
> -test:ok("PASS")<br>
> +os.exit(test:check() and 0 or 1)<br>
> diff --git a/test/fold_bug_LuaJIT_524.test.lua b/test/fold_bug_LuaJIT_524.test.lua<br>
> index b056684..c8cc7b0 100755<br>
> --- a/test/fold_bug_LuaJIT_524.test.lua<br>
> +++ b/test/fold_bug_LuaJIT_524.test.lua<br>
> @@ -21,4 +21,4 @@ end<br>
> <br>
> test:is(tonumber(sq), math.fmod(math.pow(42, 8), math.pow(2, 32)))<br>
> <br>
> -test:check()<br>
> +os.exit(test:check() and 0 or 1)<br>
> diff --git a/test/gh.test.lua b/test/gh.test.lua<br>
> index 00b71a6..2804d35 100755<br>
> --- a/test/gh.test.lua<br>
> +++ b/test/gh.test.lua<br>
> @@ -1,17 +1,17 @@<br>
> #!/usr/bin/env tarantool<br>
> <br>
> -- Miscellaneous test for LuaJIT bugs<br>
> -tap = require('tap')<br>
> +local tap = require('tap')<br>
> <br>
> -test = tap.test("gh")<br>
> +local test = tap.test("gh")<br>
> test:plan(2)<br>
> --<br>
> -- gh-3196: incorrect string length if Lua hash returns 0<br>
> --<br>
> -h = "\x1F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"<br>
> +local h = "\x1F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"<br>
> test:is(h:len(), 20)<br>
> <br>
> h = "\x0F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"<br>
> test:is(h:len(), 20)<br>
> <br>
> -test:check()<br>
> +os.exit(test:check() and 0 or 1)<br>
> diff --git a/test/pairsmm_tarantool_4560.test.lua b/test/pairsmm_tarantool_4560.test.lua<br>
> index bb9835d..f8cf0bb 100755<br>
> --- a/test/pairsmm_tarantool_4560.test.lua<br>
> +++ b/test/pairsmm_tarantool_4560.test.lua<br>
> @@ -1,8 +1,8 @@<br>
> #!/usr/bin/env tarantool<br>
> <br>
> -tap = require('tap')<br>
> +local tap = require('tap')<br>
> <br>
> -test = tap.test("PAIRSMM-is-set")<br>
> +local test = tap.test("PAIRSMM-is-set")<br>
> test:plan(2)<br>
> <br>
> -- There is no Lua way to detect whether LJ_PAIRSMM is enabled. However, in<br>
> @@ -46,5 +46,7 @@ for k, v in ipairs(t) do<br>
> ipairs_res = v + ipairs_res<br>
> end<br>
> test:is(pairs_res + ipairs_res, 0)<br>
> -os_exec_res = os.execute()<br>
> +local os_exec_res = os.execute()<br>
> test:is(os_exec_res, 1)<br>
> +<br>
> +os.exit(test:check() and 0 or 1)<br>
> diff --git a/test/table_chain_bug_LuaJIT_494.test.lua b/test/table_chain_bug_LuaJIT_494.test.lua<br>
> index 06c0f0d..85df8e0 100755<br>
> --- a/test/table_chain_bug_LuaJIT_494.test.lua<br>
> +++ b/test/table_chain_bug_LuaJIT_494.test.lua<br>
> @@ -1,8 +1,8 @@<br>
> #!/usr/bin/env tarantool<br>
> <br>
> -tap = require('tap')<br>
> +local tap = require('tap')<br>
> <br>
> -test = tap.test("494")<br>
> +local test = tap.test("494")<br>
> test:plan(1)<br>
> <br>
> -- Test file to demonstrate Lua table hash chain bugs discussed in<br>
> @@ -11,19 +11,19 @@ test:plan(1)<br>
> -- <a href="https://gist.github.com/corsix/1fc9b13a2dd5f3659417b62dd54d4500" target="_blank" rel=" noopener noreferrer">https://gist.github.com/corsix/1fc9b13a2dd5f3659417b62dd54d4500</a><br>
> <br>
> --- Plumbing<br>
> -ffi = require"ffi"<br>
> -ffi.cdef"char* strstr(const char*, const char*)"<br>
> -strstr = ffi.C.strstr<br>
> -cast = ffi.cast<br>
> -str_hash_offset = cast("uint32_t*", strstr("*", ""))[-2] == 1 and 3 or 2<br>
> +local ffi = require("ffi")<br>
> +ffi.cdef("char* strstr(const char*, const char*)")<br>
> +local strstr = ffi.C.strstr<br>
> +local cast = ffi.cast<br>
> +local str_hash_offset = cast("uint32_t*", strstr("*", ""))[-2] == 1 and 3 or 2<br>
> function str_hash(s)<br>
> return cast("uint32_t*", strstr(s, "")) - str_hash_offset<br>
> end<br>
> -table_new = require"table.new"<br>
> +local table_new = require("table.new")<br>
> <br>
> --- Prepare some objects<br>
> -victims = {}<br>
> -orig_hash = {}<br>
> +local victims = {}<br>
> +local orig_hash = {}<br>
> for c in ("abcdef"):gmatch"." do<br>
> v = c .. "{09add58a-13a4-44e0-a52c-d44d0f9b2b95}"<br>
> victims[c] = v<br>
> @@ -174,5 +174,6 @@ collectgarbage()<br>
> for c, v in pairs(victims) do<br>
> str_hash(v)[0] = orig_hash[c]<br>
> end<br>
> +test:ok(true, "LuaJIT/LuaJIT gh-494 successfully checked commit a7dc9e8d23315217c9fe0029bc8ae12c03306b33")<br>
> <br>
> -test:ok("PASS")<br>
> +os.exit(test:check() and 0 or 1)<br>
> diff --git a/test/unsink_64_kptr.test.lua b/test/unsink_64_kptr.test.lua<br>
> index <span class="js-phone-number_mailru_css_attribute_postfix"><span class="js-phone-number">8995763</span></span>..d01ddc6 100755<br>
> --- a/test/unsink_64_kptr.test.lua<br>
> +++ b/test/unsink_64_kptr.test.lua<br>
> @@ -1,8 +1,8 @@<br>
> #!/usr/bin/env tarantool<br>
> <br>
> -tap = require('tap')<br>
> +local tap = require('tap')<br>
> <br>
> -test = tap.test("232")<br>
> +local test = tap.test("232")<br>
> test:plan(1)<br>
> <br>
> --- From: Thibault Charbonnier <<a href="//e.mail.ru/compose/?mailto=mailto%3athibaultcha@me.com" target="_blank" rel=" noopener noreferrer">thibaultcha@me.com</a>><br>
> @@ -40,5 +40,6 @@ end<br>
> for i = 1, 1000 do<br>
> fn(i)<br>
> end<br>
> +test:ok(true, "openresty-lua/resty-core gh-232 allowed its address to be inlined")<br>
> <br>
> -test:ok("PASS")<br>
> +os.exit(test:check() and 0 or 1)<br>
> -- <br>
> 2.17.1<br>
> <br>
<br>
-- <br>
Best regards,<br>
IM<br>
</div>
</div>
</div>
</div>
</blockquote>
<br>
<br>-- <br>Alexander Tikhonov<br>
</div></div>
<base target="_self" href="https://e.mail.ru/">
</div>
</div></BODY></HTML>