From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 5CB2229D48 for ; Fri, 13 Apr 2018 04:39:33 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id qXx0ZNSTYDHH for ; Fri, 13 Apr 2018 04:39:33 -0400 (EDT) Received: from smtp40.i.mail.ru (smtp40.i.mail.ru [94.100.177.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 1B6AE296E8 for ; Fri, 13 Apr 2018 04:39:32 -0400 (EDT) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [tarantool-patches] Re: [PATCH 1/2] sql: fix tuple format leak From: "n.pettik" In-Reply-To: Date: Fri, 13 Apr 2018 11:39:22 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <0970BD07-7BA0-4EF3-B50B-1F821F7B3570@tarantool.org> References: <8d8d9f3056d4af42638ab403de6f19d2eb76f07a.1523468339.git.korablev@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy > On 12 Apr 2018, at 14:58, Vladislav Shpilevoy = wrote: >=20 > Hello. Thank you for contributing! See below 3 comments. >=20 >> diff --git a/src/box/sql.c b/src/box/sql.c >> index a6713f1f0..dd0cfcc1a 100644 >> --- a/src/box/sql.c >> +++ b/src/box/sql.c >> @@ -451,17 +451,13 @@ int tarantoolSqlite3EphemeralCreate(BtCursor = *pCur, uint32_t field_count, >> * >> * @retval SQLITE_OK on success, SQLITE_TARANTOOL_ERROR otherwise. >> */ >> -int tarantoolSqlite3EphemeralInsert(BtCursor *pCur) >> +int tarantoolSqlite3EphemeralInsert(struct space *space, char = *tuple, >> + char *tuple_end) >=20 > 1. Please, update the comment as well. And lets move it to a header, = as it is done in tarantool core. Fixed: @@ -441,18 +441,8 @@ int tarantoolSqlite3EphemeralCreate(BtCursor *pCur, = uint32_t field_count, return tarantoolSqlite3First(pCur, &unused); } =20 -/* - * Insert tuple which is contained in pX into ephemeral space. In = contrast to - * ordinary spaces, there is no need to create and fill request or = handle - * transaction routine. - * - * @param pCur Cursor pointing to ephemeral space. - * @param pX Payload containing tuple to insert. - * - * @retval SQLITE_OK on success, SQLITE_TARANTOOL_ERROR otherwise. - */ +int tarantoolSqlite3EphemeralInsert(struct space *space, const char = *tuple, + const char *tuple_end) @@ -98,8 +100,19 @@ int tarantoolSqlite3RenameParentTable(int iTab, = const char *zOldParentName, /* Interface for ephemeral tables. */ int tarantoolSqlite3EphemeralCreate(BtCursor * pCur, uint32_t = filed_count, struct coll *aColl); -int tarantoolSqlite3EphemeralInsert(struct space *space, char *tuple, - char *tuple_end); +/** + * Insert tuple into ephemeral space. + * In contrast to ordinary spaces, there is no need to create and + * fill request or handle transaction routine. + * + * @param space Ephemeral space. + * @param tuple Tuple to be inserted. + * @param tuple_end End of tuple to be inserted. + * + * @retval SQLITE_OK on success, SQLITE_TARANTOOL_ERROR otherwise. + */ +int tarantoolSqlite3EphemeralInsert(struct space *space, const char = *tuple, + const char *tuple_end); >=20 >> { >> - assert(pCur); >> - assert(pCur->curFlags & BTCF_TEphemCursor); >> - mp_tuple_assert(pCur->key, pCur->key + pCur->nKey); >> - >> - if (space_ephemeral_replace(pCur->space, pCur->key, >> - pCur->key + pCur->nKey) !=3D 0) { >> - diag_log(); >> + assert(space !=3D NULL); >> + mp_tuple_assert(tuple, tuple_end); >> + if (space_ephemeral_replace(space, tuple, tuple_end) !=3D 0) >> return SQL_TARANTOOL_INSERT_FAIL; >> - } >> return SQLITE_OK; >> } >> @@ -475,28 +471,29 @@ int tarantoolSqlite3EphemeralDrop(BtCursor = *pCur) >> } >> static inline int >> -insertOrReplace(BtCursor *pCur, enum iproto_type type) >> +insertOrReplace(struct space *space, char *tuple, char *tuple_end, >> + enum iproto_type type) >=20 > 2. Please, make a pointer be const, if it is not changed. Here it is = const. Const specifier helps compiler to do more accurate optimization. Fixed: @@ -471,7 +461,7 @@ int tarantoolSqlite3EphemeralDrop(BtCursor *pCur) } =20 static inline int -insertOrReplace(struct space *space, char *tuple, char *tuple_end, +insertOrReplace(struct space *space, const char *tuple, const char = *tuple_end, >=20 >> -int tarantoolSqlite3Insert(BtCursor *pCur) >> +int tarantoolSqlite3Insert(struct space *space, char *tuple, char = *tuple_end) >> { >> - return insertOrReplace(pCur, IPROTO_INSERT); >> + return insertOrReplace(space, tuple, tuple_end, IPROTO_INSERT); >> } >> -int tarantoolSqlite3Replace(BtCursor *pCur) >> +int tarantoolSqlite3Replace(struct space *space, char *tuple, char = *tuple_end) >=20 > 3. Same. -int tarantoolSqlite3Replace(struct space *space, char *tuple, char = *tuple_end) +int tarantoolSqlite3Replace(struct space *space, const char *tuple, + const char *tuple_end) -int tarantoolSqlite3Insert(struct space *space, char *tuple, char = *tuple_end) +int tarantoolSqlite3Insert(struct space *space, const char *tuple, + const char *tuple_end) src/box/sql/tarantoolInt.h: -int tarantoolSqlite3Insert(struct space *space, char *tuple, char = *tuple_end); -int tarantoolSqlite3Replace((struct space *space, char *tuple, char = *tuple_end); +int tarantoolSqlite3Insert(struct space *space, const char *tuple, + const char *tuple_end); +int tarantoolSqlite3Replace(struct space *space, const char *tuple, + const char *tuple_end);