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 F39656FC86; Sun, 25 Apr 2021 11:08:28 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org F39656FC86 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1619338109; bh=vc/Bx5XnnZbLp3xueEEZc1zRkkxowArO+MRJ44FN3SQ=; h=To:Cc:References:Date:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=CO84buescI+dgurHd4KxrBIOK6i4PjO1iHD+uGj9syRhWVjDOEG63EtZf8w1hJEsM QEMkYEcgcyWKvvx4PP5uyPv+l6BW/2l894pZ/+Hi6BpOOLxylgdFgvt6HsA1poiHrB L47cXbKa5NxWbEFL7V5f7WvpTRqY2pd1recZkjYI= Received: from smtp61.i.mail.ru (smtp61.i.mail.ru [217.69.128.41]) (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 1AAD36FC86 for ; Sun, 25 Apr 2021 11:08:27 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 1AAD36FC86 Received: by smtp61.i.mail.ru with esmtpa (envelope-from ) id 1laZoY-00071e-2n; Sun, 25 Apr 2021 11:08:26 +0300 To: Cyrill Gorcunov , Vladislav Shpilevoy Cc: tml References: <20210419083719.29689-1-gorcunov@gmail.com> <20210419083719.29689-2-gorcunov@gmail.com> <68c7c11c-758b-5513-de57-1150c16689d8@tarantool.org> Message-ID: <48231b42-debd-ac18-2a97-bbfa9b4aec5b@tarantool.org> Date: Sun, 25 Apr 2021 11:08:25 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.9.1 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: 4F1203BC0FB41BD9203E2ABA940B75484CC38DA61F9100E7EA239D73B4E75EF5182A05F53808504049AD3015C6CE7F25E7A8DB7CFA16D634CA183C2E9AE09E02B1E45FCD5187107D X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7D4A169723F56FEDEEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006375E5376F82D9E62E48638F802B75D45FF914D58D5BE9E6BC1A93B80C6DEB9DEE97C6FB206A91F05B288A8E6745CB7AB6112E2BB4DFA8006773C743118508A179DD2E47CDBA5A96583C09775C1D3CA48CFE478A468B35FE767117882F4460429724CE54428C33FAD30A8DF7F3B2552694AC26CFBAC0749D213D2E47CDBA5A9658378DA827A17800CE7A4F00A9E8C511CEB9FA2833FD35BB23DF004C90652538430302FCEF25BFAB3454AD6D5ED66289B5278DA827A17800CE7829ABD84C861444CD32BA5DBAC0009BE395957E7521B51C20BC6067A898B09E4090A508E0FED6299176DF2183F8FC7C0DDF275FCE8193C8ECD04E86FAF290E2D7E9C4E3C761E06A71DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B65D673F6B91EFC66D089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A5A892D5D502825DF2CBC8E3D75238D4D7CE98B6D78D66AAAAD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7502E6951B79FF9A3F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D345C110A855FC099998456454942E27B39B1371E28E732BB8E4155D8AD0444D3ED6856FF249FFA92411D7E09C32AA3244CA7E3950F9BEFD84ECC850F16968E69394DBEAD0ED6C55A80FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj08M52wfuxcEeoBtZFwngSw== X-Mailru-Sender: 583F1D7ACE8F49BDD2846D59FC20E9F8E6B615FD89C6C66DFB2046E56837CBA9772A25DE7238B651424AE0EB1F3D1D21E2978F233C3FAE6EE63DB1732555E4A8EE80603BA4A5B0BC112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH 1/3] wal: sanitize wal_mode 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 Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 24.04.2021 00:17, Cyrill Gorcunov via Tarantool-patches пишет: > On Fri, Apr 23, 2021 at 10:57:11PM +0200, Vladislav Shpilevoy wrote: >> Hi! Thanks for the patch! >> >>> diff --git a/src/box/wal.h b/src/box/wal.h >>> index a8a16ba2e..470aa1cc8 100644 >>> --- a/src/box/wal.h >>> +++ b/src/box/wal.h >>> @@ -41,7 +41,29 @@ struct fiber; >>> struct wal_writer; >>> struct tt_uuid; >>> >>> -enum wal_mode { WAL_NONE = 0, WAL_WRITE, WAL_FSYNC, WAL_MODE_MAX }; >>> +enum wal_mode { >>> + /** >>> + * Do not write data at all. >>> + */ >>> + WAL_NONE, >>> + >>> + /** >>> + * Write without waiting the data to be >>> + * flushed to the storage device. >>> + */ >>> + WAL_WRITE, >>> + >>> + /** >>> + * Write data and wait the record to be >>> + * flushed to the storage device. >>> + */ >>> + WAL_FSYNC, >>> + >>> + WAL_MODE_MAX >>> +}; >>> + >>> +/** String constants for the supported modes. */ >>> +extern const char *wal_mode_STRS[WAL_MODE_MAX]; >> 1. Why do you need to declare in the header the array size? >> The old version with [] was just fine. And less places to change >> if becomes necessary. > If the size is known better to point it explicitly. Thus when you > read this code you will notise the array size (yes, it doesn't > matter from compiler pov if we declare it as a double pointer > but still). But fine I'll leave this declaration untouched. > >> 2. Please, keep the global variable where it was. We usually group >> global variable declarations. It was grouped with wal_dir_lock >> below. > --- > From: Cyrill Gorcunov > Date: Fri, 16 Apr 2021 15:54:52 +0300 > Subject: [PATCH] wal: sanitize wal_mode > > - there is no need to set WAN_NONE to 0 explicitly > - add comments about modes > - there is no need to carry NULL in wal_mode_STRS > - use designated assignment in wal_mode_STRS > > Signed-off-by: Cyrill Gorcunov > --- > src/box/wal.c | 6 +++++- > src/box/wal.h | 21 ++++++++++++++++++++- > 2 files changed, 25 insertions(+), 2 deletions(-) > > diff --git a/src/box/wal.c b/src/box/wal.c > index 5b6200b81..6468df884 100644 > --- a/src/box/wal.c > +++ b/src/box/wal.c > @@ -55,7 +55,11 @@ enum { > WAL_FALLOCATE_LEN = 1024 * 1024, > }; > > -const char *wal_mode_STRS[] = { "none", "write", "fsync", NULL }; > +const char *wal_mode_STRS[WAL_MODE_MAX] = { > + [WAL_NONE] = "none", > + [WAL_WRITE] = "write", > + [WAL_FSYNC] = "fsync", > +}; > > int wal_dir_lock = -1; > > diff --git a/src/box/wal.h b/src/box/wal.h > index a8a16ba2e..36fcd39bd 100644 > --- a/src/box/wal.h > +++ b/src/box/wal.h > @@ -41,7 +41,26 @@ struct fiber; > struct wal_writer; > struct tt_uuid; > > -enum wal_mode { WAL_NONE = 0, WAL_WRITE, WAL_FSYNC, WAL_MODE_MAX }; > +enum wal_mode { > + /** > + * Do not write data at all. > + */ > + WAL_NONE, > + > + /** > + * Write without waiting the data to be > + * flushed to the storage device. > + */ > + WAL_WRITE, > + > + /** > + * Write data and wait the record to be > + * flushed to the storage device. > + */ > + WAL_FSYNC, > + > + WAL_MODE_MAX > +}; > Thanks for the patch! I'd leave an explicit WAL_NONE = 0. I didn't know that enums start at zero, TBH. This would be more readable IMO. Up to you. > enum { > /** -- Serge Petrenko