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 E14C86EC58; Thu, 18 Feb 2021 00:46:00 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E14C86EC58 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1613598360; bh=WAYKWod8Xbk4zMdR4RH8SQgvjEXtwiZ9Ubh8nhx5Rxs=; 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=tT7dLHAGfNWUsO5EA7yGtw8xFkcgcexkn9ymkyVJ4GVzKgAXp2zK01EslYDCYdbJI goraB60hgp4luFtFYLNF0mWdvvW5m8k7MDyFhs97tRt1qAPhAXxcdLgnTZX9oRKRDz 0TyzztXRNvIKdm6T+A3J+p+zoBFCjAgK2kAoCiLA= Received: from mail-lf1-f49.google.com (mail-lf1-f49.google.com [209.85.167.49]) (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 7CD3F6EC58 for ; Thu, 18 Feb 2021 00:45:59 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7CD3F6EC58 Received: by mail-lf1-f49.google.com with SMTP id h125so125998lfd.7 for ; Wed, 17 Feb 2021 13:45:59 -0800 (PST) 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=k0Onvo5NyI5L3ICds8HvLeIjwSW4/aMQXCSm1w8weK8=; b=LLMk/sRFnb+KL0pbS4CUm0lV6qp+qBfQm8wQxY7ONUmsFKdbQtWhM1ZPuSRyXYUgRK 9PxjPvMSN+FUxo8DpSFkChY7Yl1an7QKrgdUWOKMx5GjTFUGyS/GMupB1SzBDXAQyjo6 sq8lSYdl8mdR4n5aqrstaybswY43u/V4My2sg5gPMcZeq6KMkEPt3CrNMVRw1IrlFwDT zTACCNna3Nzpelg0w0jXgsnS/dR1DUO+Vrx9frhEtGeJ0R20UJiFCyz0lKTzvplXgqFY JnDxcV7b0k2J5Zy4axcvdCi0WWZ0TjIsXxjfcWe5R2VHA2/ZhMUI6v4/QshKyrnma8eQ uNCA== X-Gm-Message-State: AOAM533OBEhvY++pw//mFQvstbxwrzF0nLvj6KNBlVBecmbo5BA17Uod VWPeJFDOc//66aG5VeJD0pMhkzKfV6k= X-Google-Smtp-Source: ABdhPJxrzuAMRIdm8cjad7J98FIdKio4qPXOAjGFqAqywJnVL6MFOcctueXIwYoI8DZEZH9ZSlEcQA== X-Received: by 2002:a19:c306:: with SMTP id t6mr533256lff.113.1613598358481; Wed, 17 Feb 2021 13:45:58 -0800 (PST) Received: from grain.localdomain ([5.18.171.94]) by smtp.gmail.com with ESMTPSA id u26sm382546lfk.148.2021.02.17.13.45.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Feb 2021 13:45:57 -0800 (PST) Received: by grain.localdomain (Postfix, from userid 1000) id B5375560132; Thu, 18 Feb 2021 00:45:56 +0300 (MSK) Date: Thu, 18 Feb 2021 00:45:56 +0300 To: Vladislav Shpilevoy Message-ID: References: <20210215140149.194727-1-gorcunov@gmail.com> <21a79be2-2af3-9eab-588f-c110aecfd0b6@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <21a79be2-2af3-9eab-588f-c110aecfd0b6@tarantool.org> User-Agent: Mutt/2.0.5 (2021-01-21) Subject: Re: [Tarantool-patches] [PATCH] txn: convert flags to explicit bitfield 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 Wed, Feb 17, 2021 at 10:15:30PM +0100, Vladislav Shpilevoy wrote: > Hi! Thanks for the patch! > > > @@ -535,7 +535,7 @@ txn_complete_fail(struct txn *txn) > > txn_rollback_one_stmt(txn, stmt); > > if (txn->engine != NULL) > > engine_rollback(txn->engine, txn); > > - if (txn_has_flag(txn, TXN_HAS_TRIGGERS)) > > + if (txn->flags & TXN_HAS_TRIGGERS) > > 1. Did you do a self-review before sending this? Can you > tell yourself what is wrong here? Yes I did, and I don't get what's wrong with this line? Previously we have `if (txn_has_flag(txn, TXN_HAS_TRIGGERS))` where txn_has_flag get expanded to return (txn->flags & TXN_HAS_TRIGGERS) != 0; so where is an error? > 2. What was wrong with the idea of having the helpers > but operating on bitfields? And having has_flags() instead > of has_flag() to check presense of all specified flags + > clear_flags() to remove all specified flags. > > I don't understand. I think we discussed it already 2 or 3 > times and all seemed to agree? The initial idea was to operate with flags directly, you pointed that lines of code as if (!(txn->flags & mask)) violates our code style. Then we thought of renaming the helpers to txn_x_flags to operate with multiple flags, which lead us to a vague moment -- it is unclear how exactly txn_has_flag should operate: exact matching or any matching? Finlly I simply moved back to direct access to the fields with explicit `!= 0` or `== 0` form.