From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp29.i.mail.ru (smtp29.i.mail.ru [94.100.177.89]) (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 43A7E45C304 for ; Thu, 17 Dec 2020 19:41:01 +0300 (MSK) Date: Thu, 17 Dec 2020 19:41:09 +0300 From: Alexander Turenko Message-ID: <20201217164109.xpzksjearubo5lnc@tkn_work_nb> References: <20201214153527.451373-1-void@tarantool.org> <06ed17cb-9758-33fc-25da-61db02d1f60b@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH] lua/key_def: fix compare_with_key() part count check List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Nikiforov Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy On Thu, Dec 17, 2020 at 04:27:09PM +0300, Sergey Nikiforov wrote: > > > diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c > > > index a781aeff9..674891a85 100644 > > > --- a/src/box/lua/key_def.c > > > +++ b/src/box/lua/key_def.c > > > @@ -362,6 +362,15 @@ lbox_key_def_compare_with_key(struct lua_State *L) > > > size_t key_len; > > > const char *key_end, *key = lbox_encode_tuple_on_gc(L, 3, &key_len); > > > uint32_t part_count = mp_decode_array(&key); > > > + > > > + if (part_count > key_def->part_count) { > > > + region_truncate(region, region_svp); > > > + tuple_unref(tuple); > > > + diag_set(ClientError, ER_KEY_PART_COUNT, > > > + key_def->part_count, part_count); > > > + return luaT_error(L); > > > + } > > > > Why this check and the call below can't be all simply > > replaces with box_key_def_validate_key() call? > > Because we need part_count later. With box_key_def_validate_key() we would > have to call mp_decode_array() twice or add yet another parameter to > box_key_def_validate_key(). Is that good idea? We can't change box_key_def_validate_key() parameters, it is in the public C API. The code would look more accurate if we'll reuse the public functions (box_key_def_validate_key() and box_tuple_compare_with_key()) here. However, right, it'll decode the msgpack array size twice. I think it is negligible comparing to the validation of the key against given key_def. If'll need maximum performance from the module, we'll add an option to skip validation at all. > > > > + > > > if (key_validate_parts(key_def, key, part_count, true, > > > &key_end) != 0) { > > > region_truncate(region, region_svp);