From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: Kirill Shcherbatov <kshcherbatov@tarantool.org>, tarantool-patches@freelists.org Subject: [tarantool-patches] Re: [PATCH v2 05/11] box: port schema_find_id to C Date: Wed, 13 Jun 2018 21:53:13 +0300 [thread overview] Message-ID: <e43082a7-c4d6-8800-4c73-0805736d4766@tarantool.org> (raw) In-Reply-To: <cbaf3acc6bac97cf6347e0e04efc5d2a3eebbc1c.1528535873.git.kshcherbatov@tarantool.org> Thanks for the patch! See 3 comments below. On 09/06/2018 12:32, Kirill Shcherbatov wrote: > Part of #3273. > --- > src/box/schema.cc | 54 ++++++++++++++++++++++++++++++++++++++---------------- > src/box/schema.h | 23 ++++++++++++++++------- > src/box/user.cc | 4 +++- > 3 files changed, 57 insertions(+), 24 deletions(-) > > diff --git a/src/box/schema.cc b/src/box/schema.cc > index 2ddf920..5d32e61 100644 > --- a/src/box/schema.cc > +++ b/src/box/schema.cc > @@ -222,30 +222,52 @@ sc_space_new(uint32_t id, const char *name, struct key_def *key_def, > trigger_run_xc(&on_alter_space, space); > } > > -uint32_t > +int > schema_find_id(uint32_t system_space_id, uint32_t index_id, > - const char *name, uint32_t len) > + const char *name, uint32_t len, uint32_t *object_id) > { > + if (rc == 0) { > /* id is always field #1 */ > - return tuple_field_u32_xc(tuple, 0); > + if (tuple == NULL) > + *object_id = BOX_ID_NIL; > + else if (tuple_field_u32(tuple, 0, object_id) != 0) > + return -1; 1. You should not apply my diff as is with no inspection. Usually I do draft fixes in review. Here is the bug - on return -1 the iterator and region leaks. This time apply this: else if (tuple_field_u32(tuple, 0, object_id) != 0) - return -1; + rc = -1; } > diff --git a/src/box/schema.h b/src/box/schema.h > index 1f7414f..cd9bb5b 100644 > --- a/src/box/schema.h > +++ b/src/box/schema.h > @@ -102,6 +102,22 @@ space_is_system(struct space *space); > struct sequence * > sequence_by_id(uint32_t id); > > +/** > + * Find space id by name in specified system space with index. 2. Same as on the previous review: this function searches not only space id, but a system object id. > + * > + * @param system_space_id identifier of the system space. > + * @param index_id identifier of the index to lookup. > + * @param name of object to lookup. > + * @param len length of a name. > + * @param object_id[out] object_id or BOX_ID_NIL - not found. > + * > + * @retval 0 on success. > + * @retval -1 on error. > + */ > +int > +schema_find_id(uint32_t system_space_id, uint32_t index_id, > + const char *name, uint32_t len, uint32_t *object_id); 3. Invalid alignment. > + > #if defined(__cplusplus) > } /* extern "C" */ >
next prev parent reply other threads:[~2018-06-13 18:53 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-06-09 9:58 [tarantool-patches] [PATCH v2 00/11] sql: remove Triggers to server Kirill Shcherbatov 2018-06-09 9:32 ` [tarantool-patches] [PATCH v2 10/11] sql: move " Kirill Shcherbatov 2018-06-13 18:53 ` [tarantool-patches] " Vladislav Shpilevoy 2018-06-14 16:12 ` Kirill Shcherbatov 2018-06-28 7:18 ` Konstantin Osipov 2018-06-28 7:33 ` Kirill Shcherbatov 2018-06-09 9:32 ` [tarantool-patches] [PATCH v2 11/11] sql: VDBE tests for trigger existence Kirill Shcherbatov 2018-06-13 18:53 ` [tarantool-patches] " Vladislav Shpilevoy 2018-06-14 16:12 ` Kirill Shcherbatov 2018-06-09 9:32 ` [tarantool-patches] [PATCH v2 02/11] box: move db->pShchema init to sql_init Kirill Shcherbatov 2018-06-09 9:32 ` [tarantool-patches] [PATCH v2 04/11] sql: fix sql len in tarantoolSqlite3RenameTrigger Kirill Shcherbatov 2018-06-09 9:32 ` [tarantool-patches] [PATCH v2 06/11] sql: refactor sql_expr_compile to return AST Kirill Shcherbatov 2018-06-13 18:53 ` [tarantool-patches] " Vladislav Shpilevoy 2018-06-14 16:12 ` Kirill Shcherbatov 2018-06-09 9:32 ` [tarantool-patches] [PATCH v2 07/11] sql: move sqlite3DeleteTrigger to sql.h Kirill Shcherbatov 2018-06-13 18:53 ` [tarantool-patches] " Vladislav Shpilevoy 2018-06-14 16:12 ` Kirill Shcherbatov 2018-06-09 9:32 ` [tarantool-patches] [PATCH v2 08/11] box: sort error codes in misc.test Kirill Shcherbatov 2018-06-09 9:32 ` [tarantool-patches] [PATCH v2 09/11] sql: new _trigger space format with space_id Kirill Shcherbatov 2018-06-13 18:53 ` [tarantool-patches] " Vladislav Shpilevoy 2018-06-14 16:12 ` Kirill Shcherbatov 2018-06-09 9:58 ` [tarantool-patches] [PATCH v2 01/11] box: remove migration from alpha 1.8.2 and 1.8.4 Kirill Shcherbatov 2018-06-09 9:58 ` [tarantool-patches] [PATCH v2 03/11] sql: fix leak on CREATE TABLE and resolve self ref Kirill Shcherbatov 2018-06-13 18:53 ` [tarantool-patches] " Vladislav Shpilevoy 2018-06-14 16:12 ` Kirill Shcherbatov 2018-06-09 9:58 ` [tarantool-patches] [PATCH v2 05/11] box: port schema_find_id to C Kirill Shcherbatov 2018-06-13 18:53 ` Vladislav Shpilevoy [this message] 2018-06-14 16:12 ` [tarantool-patches] " Kirill Shcherbatov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=e43082a7-c4d6-8800-4c73-0805736d4766@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=kshcherbatov@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH v2 05/11] box: port schema_find_id to C' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox