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 EB68B22916 for ; Tue, 5 Feb 2019 10:42:06 -0500 (EST) 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 bGgEC_4vTInC for ; Tue, 5 Feb 2019 10:42:06 -0500 (EST) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 959EA1FC3A for ; Tue, 5 Feb 2019 10:42:06 -0500 (EST) Subject: [tarantool-patches] Re: [PATCH] box: fix bug with module_reload() without box.cfg{} References: <20190205001737.8475-1-roman.habibov@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Tue, 5 Feb 2019 18:41:53 +0300 MIME-Version: 1.0 In-Reply-To: <20190205001737.8475-1-roman.habibov@tarantool.org> 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, Roman Khabibov Hi! Thanks for the patch! See 5 comments below. On 05/02/2019 01:17, Roman Khabibov wrote: > Fix bug with segfault when use module_reload() before box.cfg{}. 1. Instead of duplicating commit title, it is usually better to describe here a reason why the bug existed. > > Closes #3770 > > Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3770-module-reload > Issue: https://github.com/tarantool/tarantool/issues/3770 2. Please, put branch and issue links after '---'. > --- Here. > src/box/func.c | 3 +++ > test/box/func_reload.result | 9 +++++++++ > test/box/func_reload.test.lua | 5 +++++ > 3 files changed, 17 insertions(+) > > diff --git a/src/box/func.c b/src/box/func.c > index a817851fd..ee6dd55dc 100644 > --- a/src/box/func.c > +++ b/src/box/func.c > @@ -301,6 +301,9 @@ module_sym(struct module *module, const char *name) > int > module_reload(const char *package, const char *package_end, struct module **module) > { > + if (!modules) > + return -1; 3. As you know, we use == NULL instead of ! when dealing with pointers. 4. Instead of checking for modules != NULL in each public func.h function, it is better to move module_init() call from box_cfg_xc() to box_init(). Then main() will create modules hash. > + > struct module *old_module = module_cache_find(package, package_end); > if (old_module == NULL) { > /* Module wasn't loaded - do nothing. */ > diff --git a/test/box/func_reload.test.lua b/test/box/func_reload.test.lua > index 8906898ec..00fe8cf48 100644 > --- a/test/box/func_reload.test.lua > +++ b/test/box/func_reload.test.lua > @@ -1,3 +1,8 @@ > +-- gh-3770 Check no segfault with module_reload() without box.cfg{}. > + > +box.internal.module_reload('') > +box.internal.module_reload('xxx') 5. Not-tap tests are always run with already called box.cfg(), so these lines here will work regardless of your patch. Please, move it to a tap test, in app-tap directory. Now I do not see there any existing and suitable file for this test, so you should create a new one. For example, app-tap/func.test.lua. > + > fio = require('fio') > net = require('net.box') > fiber = require('fiber') > -- > 2.17.1 > >