[tarantool-patches] Re: [tarantool-patches] Re: [PATCH] Proper error handling for fio.mktree

[Maria Khaydich]

Hello, 

sorry for the delayed reply and thank you for the feedback.
As Georgy had stated, fixed version was already pushed.

Regarding the commit message, I'm pretty sure you were wrong 
about 2 and 4, whereas 3 is debatable. Thanks anyway though.


>Вторник, 17 сентября 2019, 15:18 +03:00 от Kirill Yukhin <kyukhin at tarantool.org>:
>
>Hello,
>
>On 17 Sep 11:23, Maria wrote:
>> Method fio.mktree used to create given path
>
>Nit: is used
>
>> unconditionally - without checking whether
>> it was a directory or something else. This
>
>Nit: if it was
>
>> way led to inappropriate error messages or
>
>Nit: This led
>
>> even inconsistent behavior. Now we check the
>
>Nit: Now check
>
>> type of a given path.
>> 
>> Closes #4439
>> 
>> Issue:
>>  https://github.com/tarantool/tarantool/issues/4439
>> Branch:
>>  https://github.com/tarantool/tarantool/tree/eljashm/gh-4439-error-handling-fio.mktree
>> ---
>>  src/lua/fio.lua       | 8 +++++++-
>>  test/app/fio.test.lua | 6 ++++++
>>  2 files changed, 13 insertions(+), 1 deletion(-)
>> 
>> diff --git a/src/lua/fio.lua b/src/lua/fio.lua
>> index 321ae8b2d..a3a8ddc10 100644
>> --- a/src/lua/fio.lua
>> +++ b/src/lua/fio.lua
>> @@ -360,14 +361,19 @@ fio.mktree = function(path, mode)
>>      local current_dir = "/"
>>      for i, dir in ipairs(dirs) do
>>          current_dir = fio.pathjoin(current_dir, dir)
>> -        if not fio.stat(current_dir) then
>> +        local stat = fio.stat(current_dir)
>> +        if stat == nil then
>>              local st, err = fio.mkdir(current_dir, mode)
>>              if err ~= nil  then
>>                  return false, string.format("Error creating directory %s: %s",
>>                      current_dir, tostring(err))
>>              end
>> +        elseif not stat:is_dir() then
>> +            return false, string.format("Error creating directory %s: %s",
>> +                current_dir, errno.strerror(errno.EEXIST))
>>          end
>>      end
>> +
>>      return true
>
>This is useless change. Please discard.
>
>> diff --git a/test/app/fio.test.lua b/test/app/fio.test.lua
>> index be735e4f3..56c957d8a 100644
>> --- a/test/app/fio.test.lua
>> +++ b/test/app/fio.test.lua
>> @@ -468,3 +468,9 @@ fio.rmtree(1)
>>  fio.copytree(nil, nil)
>>  fio.copytree(nil, nil)
>>  test_run:cmd("clear filter")
>> +
>> +--
>> +-- gh-4439: mktree error handling fix - creation of existing file/directory
>> +--
>> +fio.mktree('/dev/null')
>> +fio.mktree('/dev/null/dir')
>> -- 
>> 2.20.1 (Apple Git-117)
>
>Please update .result file as well.
>
>Otherwise, lgtm.
>
>--
>Regards, Kirill Yukhin
>


-- 
Maria Khaydich
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20190925/7d89b2d8/attachment.html>