Re: [Tarantool-patches] [PATCH v2 05/10] box/error: don't set error created via box.error.new to diag

[Nikita Pettik <korablev@tarantool.org>]

On 26 Mar 17:59, Nikita Pettik wrote:
> On 26 Mar 19:50, Konstantin Osipov wrote:
> > * Nikita Pettik <korablev@tarantool.org> [20/03/25 09:32]:
> > > To achieve this let's refactor luaT_error_create() to return error
> > > object instead of setting it via box_error_set().
> > > luaT_error_create() is used both to handle box.error() and
> > > box.error.new() invocations, and box.error() is still expected to set
> > > error to diagnostic area. So, luaT_error_call() which implements
> > > box.error() processing at the end calls diag_set_error().
> > > It is worth mentioning that net.box module relied on the fact that
> > > box.error.new() set error to diagnostic area: otherwise request errors
> > > don't get to diagnostic area on client side.
> > > 
> > 
> > I know there was a discussion about construct vs create vs new.
> 
> It was not really discussion, I accidentally didn't notice that
> box_error_construct() in fact allocated memory (meanwhile I thought
> it didn't). I've already renamed it to box_error_new().
> 
> > Let me remind about approved name pairs:
> > 
> > obj_create/obj_destroy - constructor/destructor, without
> > allocation/deallocation.
> > 
> > obj_new/obj_delete - constructor/destructor with
> > allocation/deallocation
> > 
> > init/free - library initialization/destruction
> > 
> > _construct is not in the vocabulary. I'd avoid it.
> 
> Ok, Vlad already guided me (but it would be nice to see this
> manual in our dev docs).

Oh, sorry, my bad. It is documented convention:
tarantool.io/en/doc/1.10/dev_guide/c_style_guide/

> > I know this is bikeshed, pls feel free to ignore in scope of this
> > patch.  And sorry for jumping in late.
> > 
> > > +struct error *
> > > +box_error_construct(const char *file, unsigned line, uint32_t code,
> > > +		    const char *fmt, ...)
> > > +{
> > > +	struct error *e = BuildClientError(file, line, ER_UNKNOWN);
> > > +	ClientError *client_error = type_cast(ClientError, e);
> > > +	if (client_error != NULL) {
> > > +		client_error->m_errcode = code;
> > > +		va_list ap;
> > > +		va_start(ap, fmt);
> > > +		error_vformat_msg(e, fmt, ap);
> > > +		va_end(ap);
> > > +	}
> > > +	return e;
> > > +}
> > 
> > if box_error_new() is already taken by the public api, options are
> > box_client_error_new() or box_build_client_error()?
> 
> No, box_error_new() name is still available. I've renamed
> box_error_construct() to box_error_new() and pushed branch
> containing this changes.
> 
> > 
> > -- 
> > Konstantin Osipov, Moscow, Russia