Re: [Tarantool-patches] [PATCH luajit v2 2/7] core: introduce write buffer module

[Sergey Ostanevich <sergos@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 1761 bytes --]

Hi! 

Thanks for the patch!

Just one problem and LGTM otherwise.

Sergos

> --- a/src/lj_wbuf.h
> +++ b/src/lj_wbuf.h
> @@ -31,32 +31,32 @@
> #define STREAM_ERR_IO 0x1
> #define STREAM_STOP   0x2
> 
> +/*
> +** Buffer writer which is called on the buffer flush.
> +** Should return amount of written bytes on success or zero in case of error.
> +** *data should contain a buffer of at least the initial size.

Does it mean the writer’s provider should preserve the ‘initial size’ is some way?
Should it create two or more buffers of the same size during initialization?

> +** If *data == NULL stream stops.
> +*/
> typedef size_t (*lj_wbuf_writer)(const void **data, size_t len, void *opt);
> 
> /* Write buffer. */
> struct lj_wbuf {
> -  /*
> -  ** Buffer writer which will called at buffer write.
> -  ** Should return amount of written bytes on success or zero in case of error.
> -  ** *data should contain new buffer of size greater or equal to len.
> -  ** If *data == NULL stream stops.
> -  */
>   lj_wbuf_writer writer;
>   /* Context for writer function. */
>   void *ctx;
>   /* Buffer size. */
>   size_t size;
> -  /* Saved errno in case of error. */
> -  int saved_errno;
>   /* Start of buffer. */
>   uint8_t *buf;
>   /* Current position in buffer. */
>   uint8_t *pos;
> +  /* Saved errno in case of error. */
> +  int saved_errno;
>   /* Internal flags. */
>   volatile uint8_t flags;
> };
> 
> -/* Init buffer. */
> +/* Initialize the buffer. */
> void lj_wbuf_init(struct lj_wbuf *buf, lj_wbuf_writer writer, void *ctx,
> 		  uint8_t *mem, size_t size);
> 
> ===================================================================
> 
> -- 
> Best regards,
> Sergey Kaplun


[-- Attachment #2: Type: text/html, Size: 36531 bytes --]