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

[Nikita Pettik <korablev@tarantool.org>]

On 26 Mar 01:22, Vladislav Shpilevoy wrote:
> On 25/03/2020 02:02, Nikita Pettik wrote:
> > On 22 Feb 18:18, Vladislav Shpilevoy wrote:
> >> Thanks for the patch!
> >>
> >>> diff --git a/src/box/error.h b/src/box/error.h
> >>> index b8c7cf73d..42043ef80 100644
> >>> --- a/src/box/error.h
> >>> +++ b/src/box/error.h
> >>> @@ -129,6 +129,14 @@ int
> >>>  box_error_set(const char *file, unsigned line, uint32_t code,
> >>>  	      const char *format, ...);
> >>>  
> >>> +/**
> >>> + * Construct error object without setting it in the diagnostics
> >>> + * area. On the memory allocation fail returns NULL.
> >>> + */
> >>> +struct error *
> >>> +box_error_construct(const char *file, unsigned line, uint32_t code,
> >>> +		    const char *fmt, ...);
> >>
> >> 5. You added the method to the public C API (it is inside 'cond public'
> >> comments). I don't think we should do that. At least until someone
> >> asked so.
> >>
> >> Also why not box_error_new()? 'Construct' seems to be a very strange name.
> > 
> > IMHO _new 'suffix' implies allocating new object, meanwhile _construct - 
> > filling in already allocated object. It was my understanding.
> Exactly. And box_error_construct() allocates a new object. You even said
> that in the comment. So where am I wrong?

Indeed. I should have paid more attention. Let's rename it to _new().
Otherwise patch looks good to you?
 
> Btw, for initialization of an object in-place we use '_create'.
> '_init' is used for initialization of a module. But '_construct' - never.

Ok, will keep in mind.