Re: [Tarantool-patches] [PATCH 06/16] cord_buf: introduce ownership management

[Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

23.03.2021 01:32, Vladislav Shpilevoy пишет:
> Hi! Thanks for the review!
>
>>> diff --git a/src/lib/core/cord_buf.c b/src/lib/core/cord_buf.c
>>> index cac508c3d..9450d75bc 100644
>>> --- a/src/lib/core/cord_buf.c
>>> +++ b/src/lib/core/cord_buf.c
>>> @@ -5,6 +5,7 @@
>>>     */
>>>    #include "cord_buf.h"
>>>    #include "fiber.h"
>>> +#include "trigger.h"
>>>      #include "small/ibuf.h"
>>>    @@ -13,35 +14,154 @@ enum {
>>>        CORD_IBUF_START_CAPACITY = 16384,
>>>    };
>>>    -static struct ibuf *cord_buf_global = NULL;
>>> +/** Global buffer with automatic collection on fiber yield. */
>>> +struct cord_buf {
>>> +    /** Base buffer. */
>>> +    struct ibuf base;
>>> +    /**
>>> +     * Triggers on fiber stop/yield when the buffer is either destroyed or
>>> +     * cached to the global stash for later reuse.
>>> +     */
>>> +    struct trigger on_stop;
>>> +    struct trigger on_yield;
>>> +#if !NDEBUG
>> Shouldn't it be `#if !defined(NDEBUG)`? I'm not sure, just asking.
> Yes, totally forgot about it.

Thanks! LGTM.
>
> ====================
> diff --git a/src/lib/core/cord_buf.c b/src/lib/core/cord_buf.c
> index 9450d75bc..1d0fb3a41 100644
> --- a/src/lib/core/cord_buf.c
> +++ b/src/lib/core/cord_buf.c
> @@ -24,7 +24,7 @@ struct cord_buf {
>   	 */
>   	struct trigger on_stop;
>   	struct trigger on_yield;
> -#if !NDEBUG
> +#ifndef NDEBUG
>   	/**
>   	 * Fiber owning the buffer right now. Used for debug and sanity checks
>   	 * only.
> @@ -52,7 +52,7 @@ cord_buf_set_owner(struct cord_buf *buf)
>   	struct fiber *f = fiber();
>   	trigger_add(&f->on_stop, &buf->on_stop);
>   	trigger_add(&f->on_yield, &buf->on_yield);
> -#if !NDEBUG
> +#ifndef NDEBUG
>   	buf->owner = f;
>   #endif
>   	ibuf_reset(&buf->base);
> @@ -64,7 +64,7 @@ cord_buf_clear_owner(struct cord_buf *buf)
>   	assert(buf->owner == fiber());
>   	trigger_clear(&buf->on_stop);
>   	trigger_clear(&buf->on_yield);
> -#if !NDEBUG
> +#ifndef NDEBUG
>   	buf->owner = NULL;
>   #endif
>   }
> @@ -98,7 +98,7 @@ cord_buf_new(void)
>   	ibuf_create(&buf->base, &cord()->slabc, CORD_IBUF_START_CAPACITY);
>   	trigger_create(&buf->on_stop, cord_buf_on_stop, buf, NULL);
>   	trigger_create(&buf->on_yield, cord_buf_on_yield, buf, NULL);
> -#if !NDEBUG
> +#ifndef NDEBUG
>   	buf->owner = NULL;
>   #endif
>   	return buf;

-- 
Serge Petrenko