From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp47.i.mail.ru (smtp47.i.mail.ru [94.100.177.107]) (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 8093E42F4AD for ; Fri, 19 Jun 2020 20:35:03 +0300 (MSK) References: <16c9d1ffb9d09bb2b2f206a23973e2734616c345.1592482315.git.sergepetrenko@tarantool.org> <4229c1b6-54e1-3c84-c902-37d49d3658c6@tarantool.org> From: Serge Petrenko Message-ID: Date: Fri, 19 Jun 2020 20:35:02 +0300 MIME-Version: 1.0 In-Reply-To: <4229c1b6-54e1-3c84-c902-37d49d3658c6@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-GB Subject: Re: [Tarantool-patches] [PATCH 4/4] txn_limbo: add ROLLBACK processing List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy , gorcunov@gmail.com Cc: tarantool-patches@dev.tarantool.org 19.06.2020 01:15, Vladislav Shpilevoy пишет: > Thanks for the patch! Thanks for the review! > >> diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c >> index a715a136e..c55f5bda1 100644 >> --- a/src/box/txn_limbo.c >> +++ b/src/box/txn_limbo.c >> @@ -84,6 +84,16 @@ txn_limbo_remove(struct txn_limbo *limbo, struct txn_limbo_entry *entry) >> rlist_del_entry(entry, in_queue); >> } >> >> +static inline void >> +txn_limbo_pop(struct txn_limbo *limbo, struct txn_limbo_entry *entry) >> +{ >> + assert(!rlist_empty(&entry->in_queue)); >> + assert(rlist_last_entry(&limbo->queue, struct txn_limbo_entry, >> + in_queue) == entry); >> + (void) limbo; >> + rlist_del_entry(entry, in_queue); >> +} > txn_limbo_remove is exactly the same as txn_limbo_pop. I suggest to keep > one of them. > > Everything else looks nice. Assertions are different. I wanted to stress  that `pop` removes entries starting from the tail, and `remove`, on the contrary,  removes them starting from the head. Looks strange, though, I agree. If  you merge the functions and put an assertion `rlist_first_entry == ... || rlist_last_entry == ...` you'll lose some strictness in their use. Feel free to decide what to do. -- Serge Petrenko