From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id E14FD6EC5B; Mon, 12 Apr 2021 01:38:04 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E14FD6EC5B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1618180684; bh=nDbowUQZlZiTj45UxYNqHaB/8XicX6pNCppgTpaOJpo=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=PbcfzNPVO1c5H5bsYzB6f1Z6pGH89b6Vptzuu7VaI+B0rXTXTeuW0/C/aznJfgQwa IjqOmrswPQRrwS/6BnrElSL1MocMCSN5UgZyVrEL0sJcD1IUyy+aVxFqYa5pU0VOeG m+Cl+7+rHD4YNawWXlYU/L6YeL/a2EGEv1UmCk9c= Received: from mail-lf1-f41.google.com (mail-lf1-f41.google.com [209.85.167.41]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 74F206EC5B for ; Mon, 12 Apr 2021 01:38:03 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 74F206EC5B Received: by mail-lf1-f41.google.com with SMTP id w8so9792897lfr.0 for ; Sun, 11 Apr 2021 15:38:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=zAFg0NiufHbCyObwIDl61GQ50oHwRJg3VMxFc4K7X3o=; b=b1ZlgObgPw8VmgeCin3tKqmNH5ubjqrlUMd4JsxaNaKXi/E9Gy8ZuIOkd3IIeMVqti L0+M05ZC+qMWZaRFRs6oGac2rULGnu2dj3c9K115W5sJLmYH6DLeZZ3rSBfa3j4Flbct uwWlUAJZe3Ojgo8TgNQD3ZP9BShxyjAz29sq7Xj+F7ZSiHUU+QE4U7bnf2btn//50Hqx NaP/ApA7sNVTpQej7rfjU/fcnV8mbfIRnzahDAuG9vspjDPRvxYlMYXQ1nwhS4tURm2C 37tkyDrqwzli0Goy8CT1/ia8NXKMijJsxkyAdUuKKzJn5kpeJZciQOqF+xZSREhlkbwL LuaA== X-Gm-Message-State: AOAM5319ITHvt5/YoDSRxj/2yaChiFJj6S0FHGORox1coypW42wblU4r 0JyTKxvxVZPolz5SdsyBJ+UkP8xIluE= X-Google-Smtp-Source: ABdhPJwr1pToMZBcexoElb2Zf2G5TLx1/1y2tmDWd3kSyOz66kTwweOB580F3jgfPgSrVZDHesqB5A== X-Received: by 2002:a05:6512:21c2:: with SMTP id d2mr16945250lft.424.1618180682293; Sun, 11 Apr 2021 15:38:02 -0700 (PDT) Received: from grain.localdomain ([5.18.199.94]) by smtp.gmail.com with ESMTPSA id z28sm2282739ljn.117.2021.04.11.15.38.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 11 Apr 2021 15:38:01 -0700 (PDT) Received: by grain.localdomain (Postfix, from userid 1000) id 7F079560112; Mon, 12 Apr 2021 01:38:00 +0300 (MSK) Date: Mon, 12 Apr 2021 01:38:00 +0300 To: Vladislav Shpilevoy Message-ID: References: <20210408164151.1759348-1-gorcunov@gmail.com> <20210408164151.1759348-6-gorcunov@gmail.com> <0868de76-3941-ed34-3d7e-2f7328e30934@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0868de76-3941-ed34-3d7e-2f7328e30934@tarantool.org> User-Agent: Mutt/2.0.5 (2021-01-21) Subject: Re: [Tarantool-patches] [PATCH v21 5/6] box: implement box.lib module X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Cyrill Gorcunov via Tarantool-patches Reply-To: Cyrill Gorcunov Cc: tml Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On Sun, Apr 11, 2021 at 05:38:51PM +0200, Vladislav Shpilevoy wrote: > Thanks for the patch! > > Please, try to respond to my comments in the original email, > explicitly. When and how you fixed them, or why did not. Not > with promises like "will do", "ok", and so on, they don't help > much. > > Otherwise I see you tend to miss some of the comments. Also I > don't remember all of my comments, so when you don't respond to > them, I need to go to my old email, and match the comments > with the places you was supposed to fix, and it takes time. > > See how Sergey and Mergen send their patches for examples. > > https://github.com/tarantool/tarantool/wiki/Code-review-procedure#during-the-review > > See 4 comments below. I fixed the change log. And here is a diff on top, please take a look. --- Subject: [PATCH] box.lib fix Signed-off-by: Cyrill Gorcunov --- src/box/lua/lib.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/src/box/lua/lib.c b/src/box/lua/lib.c index 5fb616d04..2181b9030 100644 --- a/src/box/lua/lib.c +++ b/src/box/lua/lib.c @@ -97,12 +97,22 @@ cache_put(struct box_module_func *cf) .hash = mh_strn_hash(cf->key, cf->len), .val = cf, }; - mh_int_t e = mh_strnptr_put(func_hash, &nd, NULL, NULL); + + struct mh_strnptr_node_t prev; + struct mh_strnptr_node_t *prev_ptr = &prev; + + mh_int_t e = mh_strnptr_put(func_hash, &nd, &prev_ptr, NULL); if (e == mh_end(func_hash)) { diag_set(OutOfMemory, sizeof(nd), "malloc", "box.lib: hash node"); return -1; } + + /* + * Just to make sure we haven't replaced something, + * the entries must be explicitly deleted. + */ + assert(prev_ptr == NULL); return 0; } @@ -383,7 +393,15 @@ lbox_module_load_func(struct lua_State *L) size_t sym_len; const char *sym = lua_tolstring(L, 2, &sym_len); - const size_t max_sym_len = 512; + + /* + * C standard requires at least 63 significant + * initial characters, though it advises to not + * impose limits. Lets make the max identifier + * big enough to keep longest id, which is hardly + * be bigger than 256 symbols. + */ + const size_t max_sym_len = 256; if (sym_len < 1) { diag_set(IllegalParams, fmt_noname, method); @@ -400,13 +418,12 @@ lbox_module_load_func(struct lua_State *L) * per module. The symbol (function name) is the * last part of the hash key. * - * The key's buffer should be big enough to keep - * the longest package path plus symbol name and - * the pointer. + * Make sure there is enough space for key, + * path and formatting. */ - char key[PATH_MAX + 2 * max_sym_len]; + char key[PATH_MAX + max_sym_len + 32]; snprintf(key, sizeof(key), "%p.%s.%s", - (void *)m, m->package, sym); + m, m->package, sym); size_t len = strlen(key); struct box_module_func *cf = cache_find(key, len); -- 2.30.2