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 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).
 
> 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