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 9D07B2D9A8 for ; Sat, 8 Jun 2019 08:11:21 -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 SC6cYcgUN6aY for ; Sat, 8 Jun 2019 08:11:21 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 AC9542D7C3 for ; Sat, 8 Jun 2019 08:11:20 -0400 (EDT) Date: Sat, 8 Jun 2019 15:11:17 +0300 From: Mergen Imeev Subject: [tarantool-patches] Re: [PATCH v1 4/9] sql: rework diag_set() in OP_Halt Message-ID: <20190608121117.GA22885@tarantool.org> References: <116d34dc76fbe262015c0079bba2dc00f6a40c8c.1559043251.git.imeevma@gmail.com> <0208a503-1a04-fb78-cf8b-563185c5ab42@tarantool.org> <2f731c49-b8c4-f25b-a0c8-b5dcf382b60c@tarantool.org> <0ac8fb6a-b06f-5c36-6bab-11dbcb128484@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <0ac8fb6a-b06f-5c36-6bab-11dbcb128484@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: Vladislav Shpilevoy Cc: tarantool-patches@freelists.org Thank you for review! My answer below. On Tue, Jun 04, 2019 at 09:34:45PM +0200, Vladislav Shpilevoy wrote: > > > On 03/06/2019 11:41, Imeev Mergen wrote: > > Hi! Thank you for the review! My answers below. Also, I think I'm > > going to drop this patch, since I plan to move the error code from > > p5 to p2 in the next patch. > > > > On 6/2/19 7:35 PM, Vladislav Shpilevoy wrote: > >> > >> On 28/05/2019 14:39, imeevma@tarantool.org wrote: > >>> Prior to this patch, the way to set Tarantool error in OP_Halt was > >>> too universal. It was possible to set a description of the error > >>> that does not match its errcode. > >> There was a concrete reason, why it was possible - because different > >> error codes have different arguments of various types, and the only way > >> to set an error at parsing stage is to allow to set arbitrary error > >> message to any error code. Without '...', va_arg etc. Besides, we could > >> use it to set correct line number and function name in future. Now you > >> use diag_set, which restricts us. > >> > >> So why do you need that patch? We will need to revert it when an error > >> appears requiring more than one argument, or an argument of not > >> const char * type. That will definitely happen. > > For some reason, manually creating an error message looks a bit > > wrong to me. How about creating a new macro to create such > > messages, for example, diag_prepare(), which takes the same > > arguments as diag_set() and returns the complete error message? > > Perhaps. Sounds good to me. You can do it, if you want. But I > would suggest name "diag_prepare_msg", because it won't create a > struct error object. Only create a message. > Thanks. I'm going to do this as an follow-up patch to the current patch-set.