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 38D236EC5D; Mon, 5 Apr 2021 13:13:36 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 38D236EC5D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1617617616; bh=Md3Ephy1CApargiL118m/iFhgti78akAgCX1vl0KN0Q=; h=To:References:Date:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=VdaHqrUzlfnUtJEuE1Q/NlCMs/Rc3cEKMQC1mZcUmY2fuO3wmESSVDvc0o9Jr7I+p nnFycfHmiomkBlEiCO1caqokrliIQsKlAtQboTQwjs6wvfkov1JrANUS0yO58WVkxi ODlHuqkZofoeZgIx1qvABPT6gMKtQmRNergqjQHo= Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 5E0FA6EC5D for ; Mon, 5 Apr 2021 13:13:34 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 5E0FA6EC5D Received: by smtp3.mail.ru with esmtpa (envelope-from ) id 1lTMEf-00079x-EW; Mon, 05 Apr 2021 13:13:33 +0300 To: Cyrill Gorcunov References: <20210402123420.885834-1-gorcunov@gmail.com> <20210402123420.885834-2-gorcunov@gmail.com> <01f1a611-8eb2-4d82-88f5-0f3912cbf6b0@tarantool.org> Message-ID: <95efd72d-9969-c93f-b50f-0fbc7318079c@tarantool.org> Date: Mon, 5 Apr 2021 13:13:31 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9ED7173E37F4E32947287414FD1D04A09E656A5F3377C994A182A05F538085040957C26C34AB703FEF228A55FAE8B837857D093051E252257B7AF0C2E93D6BCA6 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE749E89BD568380EECC2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE7011EB7026DD4A9BAEA1F7E6F0F101C67CDEEF6D7F21E0D1D174C73DBBBFC766418C37248141F33C28E47B0933D7992C5A3CAD35FA96C85DB389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C06030C3405640F6718941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B6D07623A0E6354027CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB8D32BA5DBAC0009BE9E8FC8737B5C22490EF8A097A92E1CE276E601842F6C81A12EF20D2F80756B5F7E9C4E3C761E06A776E601842F6C81A127C277FBC8AE2E8B02A43AC37554C1A53AA81AA40904B5D9DBF02ECDB25306B2201CA6A4E26CD07C3BBE47FD9DD3FB595F5C1EE8F4F765FC72CEEB2601E22B093A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE4930A3850AC1BE2E7355E1C53F199C2BB95B5C8C57E37DE458BEDA766A37F9254B7 X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A24A6D60772A99906F8E1CD14B953EB46DEEDD47B60253040D355D89D7DBCDD132 X-C1DE0DAB: 0D63561A33F958A5DB80F4296CFB2382E7FD6473C39057062A5724E0A8A8FE74D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7502E6951B79FF9A3F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34AC632F0BE69382F3FFE2EA791558DE487B3E5D18BA7C8D58B38C74A34F13B0D6C24ADC03F68C5CB51D7E09C32AA3244C15349E478E9FBB6A12F168C0507D4341E646F07CC2D4F3D8927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojM00ve/f+0onuvFyW9E1xyg== X-Mailru-Sender: 583F1D7ACE8F49BDD2846D59FC20E9F8B001093FD74308B4A31838A037F4354BDD52A0D2DD5064E3424AE0EB1F3D1D21E2978F233C3FAE6EE63DB1732555E4A8EE80603BA4A5B0BC112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v20 1/7] box/schema: make sure hashes are created 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: Serge Petrenko via Tarantool-patches Reply-To: Serge Petrenko Cc: tml , Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 05.04.2021 12:50, Cyrill Gorcunov пишет: > On Mon, Apr 05, 2021 at 12:28:50PM +0300, Serge Petrenko wrote: >>> diff --git a/src/box/schema.cc b/src/box/schema.cc >>> index 963278b19..89904e4d2 100644 >>> --- a/src/box/schema.cc >>> +++ b/src/box/schema.cc >>> @@ -372,6 +372,13 @@ schema_init(void) >>> funcs = mh_i32ptr_new(); >>> funcs_by_name = mh_strnptr_new(); >>> sequences = mh_i32ptr_new(); >>> + >>> + if (spaces == NULL || spaces_by_name == NULL || >>> + funcs == NULL || funcs_by_name == NULL || >>> + sequences == NULL) { >>> + panic("Can't allocate schema hashes"); >>> + } >> I've just noticed, all the mh_..._new methods will fail on a null >> dereference even before returning. So the check above doesn't >> make much sense. > This is because our lib/salad/mhash.h is simply buggy and it must be: > > 1) fixed in mhash level > 2) get rid of this macros madness and convert it to normal > library. this macros spreading is over the top already > and I don't understand why nobody cares about icache footprint > > Nevertheless we should check for x_new resuls as we do in our > other code. Let walk over spaces > > schema_init > spaces = mh_i32ptr_new(); > ... > sc_space_new > space_cache_replace(NULL, space); > mh_int_t k = mh_i32ptr_put(spaces, &node_p, ... > > which is simply nil dereferencing (once we fix bullet 1). > >> But I see that some other places have this check >> `if (smth = mh_smth_new() == NULL)` and set an error. >> >> Am I missing something? > I think the idea was that mh_x_new doesn't do nil dereference > by its own but behaves in a safe way as any library routine > should. That said I still think we should check for mh_i32ptr_new > results. But won't insist to include this patch, we can drop it > if you prefer. I see, let's keep the patch then. LGTM. -- Serge Petrenko