From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtp17.mail.ru (smtp17.mail.ru [94.100.176.154])
 (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 1E5F6445320
 for <tarantool-patches@dev.tarantool.org>;
 Thu, 16 Jul 2020 01:02:07 +0300 (MSK)
References: <1594221263-6228-1-git-send-email-alyapunov@tarantool.org>
 <1594221263-6228-14-git-send-email-alyapunov@tarantool.org>
 <85786cb3-af03-03da-0405-b137a17ce929@tarantool.org>
 <340f6b5a-4c5d-6d26-1268-da9a7d995ac5@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <b4e3e24d-a3a5-bf0d-3976-ab0b94465b2b@tarantool.org>
Date: Thu, 16 Jul 2020 00:02:05 +0200
MIME-Version: 1.0
In-Reply-To: <340f6b5a-4c5d-6d26-1268-da9a7d995ac5@tarantool.org>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Subject: Re: [Tarantool-patches] [PATCH 13/16] tx: introduce txm_story
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Aleksandr Lyapunov <alyapunov@tarantool.org>, tarantool-patches@dev.tarantool.org

>>> +}
>>> +
>>> +static struct txm_story *
>>> +txm_story_new_add_stmt(struct tuple *tuple, struct txn_stmt *stmt)
>> 2. Probably would be better to use the existing terms: old_tuple and new_tuple.
>> So the functions would be txm_story_add_new_stmt and txm_story_add_old_stmt. The
>> same in all the other places. 'add_stmt' -> 'new_stmt'. 'del_stmt' -> 'old_stmt'.
> I thought about it. I fear in terms of txm_story, when we speak about
> lifetime of a tuple it would be weird. add_stmt - is the statement that
> added the tuple. But what is new_stmt? new_psn?

Ok, lets leave it as is.

>>
>>> +{
>>> +    struct txm_story *res = txm_story_new(tuple, stmt->space->index_count);
>>> +    if (res == NULL)
>>> +        return NULL;
>>> +    res->add_stmt = stmt;
>>> +    assert(stmt->add_story == NULL);
>>> +    stmt->add_story = res;
>>> +    return res;
>>> +}
>>> +
>>> +static struct txm_story *
>>> +txm_story_new_del_stmt(struct tuple *tuple, struct txn_stmt *stmt)
>>> +{
>>> +    struct txm_story *res = txm_story_new(tuple, stmt->space->index_count);
>>> +    if (res == NULL)
>>> +        return NULL;
>>> +    res->del_stmt = stmt;
>>> +    assert(stmt->del_story == NULL);
>>> +    stmt->del_story = res;
>>> +    return res;
>>> +}
>>> +
>>> +void
>>> +txm_history_prepare_stmt(struct txn_stmt *stmt)
>>> +{
>>> +    assert(stmt->txn->psn != 0);
>>> +
>>> +    /* Move story to the past to prepared stories. */
>>> +
>>> +    struct txm_story *story = stmt->add_story;
>>> +    uint32_t index_count = story == NULL ? 0 : story->index_count;
>>> +    for (uint32_t i = 0; i < index_count; ) {
>>> ...
>>> ...
>>> ...
>>> +    }
>>> +    if (stmt->add_story != NULL)
>> 3. According to a check in the beginning of the function stmt can
>> be NULL. But then this will crash.
> No, index_count is initialized as 0 in that case and we will not enter the loop.

This check is out of the loop. So you will stumble into it.