From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp45.i.mail.ru (smtp45.i.mail.ru [94.100.177.105]) (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 AF004469719 for ; Wed, 7 Oct 2020 16:53:04 +0300 (MSK) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.1\)) From: Roman Khabibov In-Reply-To: Date: Wed, 7 Oct 2020 16:53:01 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <20200911215115.6622-1-roman.habibov@tarantool.org> <20200911215115.6622-5-roman.habibov@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v4 4/6] sql: use parser's region of "index" array List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the review. > On Oct 6, 2020, at 00:22, Vladislav Shpilevoy = wrote: >=20 > Thanks for the patch! >=20 >> diff --git a/src/box/sql/build.c b/src/box/sql/build.c >> index 6cc76bb77..d1d240315 100644 >> --- a/src/box/sql/build.c >> +++ b/src/box/sql/build.c >> @@ -2717,7 +2737,7 @@ sql_create_index(struct Parse *parse) { >>=20 >> if (tbl_name !=3D NULL) >> goto exit_create_index; >> - table_add_index(space, index); >> + sql_space_add_index(&parse->region, space, index); >=20 > In case of a fail the error is never checked anywhere. = Parse->is_aborted > stays false. Yes. See the new diff. commit 9ee07e6a5429b4fa1e26412147f161cc921093d9 Author: Roman Khabibov Date: Thu Sep 3 14:26:47 2020 +0300 sql: use parser's region of "index" array =20 Allocate memory for the "index" array of ephemeral space on the parser's region instead of a heap as it was before. Fixed a memory leak that realloc() generated. =20 Needed for #2349 diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 3b3c8099a..fd8e7ed1e 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -2228,29 +2228,52 @@ vdbe_emit_new_sec_index_id(struct Parse *parse, = uint32_t space_id, } =20 /** - * Add new index to table's indexes list. + * Add new index to ephemeral @a space's "index" array. Reallocate + * memory on @a parse's region if needed. + * * We follow convention that PK comes first in list. * + * @param parse Parsing structure. * @param space Space to which belongs given index. * @param index Index to be added to list. */ -static void -table_add_index(struct space *space, struct index *index) +static int +sql_space_add_index(struct Parse *parse, struct space *space, + struct index *index) { uint32_t idx_count =3D space->index_count; - size_t indexes_sz =3D sizeof(struct index *) * (idx_count + 1); - struct index **idx =3D (struct index **) realloc(space->index, - indexes_sz); - if (idx =3D=3D NULL) { - diag_set(OutOfMemory, indexes_sz, "realloc", "idx"); - return; - } - space->index =3D idx; + size_t size =3D 0; + struct index **idx =3D NULL; + struct region *region =3D &parse->region; + if (idx_count =3D=3D 0) { + idx =3D region_alloc_array(region, typeof(struct index = *), 1, + &size); + if (idx =3D=3D NULL) + goto alloc_error; + /* + * Reallocate each time the idx_count becomes equal to the + * power of two. + */ + } else if ((idx_count & (idx_count - 1)) =3D=3D 0) { + idx =3D region_alloc_array(region, typeof(struct index = *), + idx_count * 2, &size); + if (idx =3D=3D NULL) + goto alloc_error; + memcpy(idx, space->index, idx_count * sizeof(struct = index *)); + } + if (idx !=3D NULL) + space->index =3D idx; /* Make sure that PK always comes as first member. */ if (index->def->iid =3D=3D 0 && idx_count !=3D 0) SWAP(space->index[0], index); space->index[space->index_count++] =3D index; space->index_id_max =3D MAX(space->index_id_max, = index->def->iid);; + return 0; + +alloc_error: + diag_set(OutOfMemory, size, "region_alloc_array", "idx"); + parse->is_aborted =3D true; + return -1; } =20 int @@ -2717,9 +2740,8 @@ sql_create_index(struct Parse *parse) { sqlVdbeAddOp0(vdbe, OP_Expire); } =20 - if (tbl_name !=3D NULL) + if (tbl_name !=3D NULL || sql_space_add_index(parse, space, = index) !=3D 0) goto exit_create_index; - table_add_index(space, index); index =3D NULL; =20 /* Clean up before exiting. */