From: Alexander Turenko <alexander.turenko@tarantool.org> To: Timur Safin <tsafin@tarantool.org> Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org Subject: Re: [Tarantool-patches] [PATCH 2.X v3] module api: box_ibuf_* wrappers Date: Tue, 13 Oct 2020 21:21:45 +0300 [thread overview] Message-ID: <20201013182145.stzjxwv3zmodm6sd@tkn_work_nb> (raw) In-Reply-To: <0cc5173291c618e58934b0460256145f42c64444.1602606208.git.tsafin@tarantool.org> > create mode 100644 src/box/box_ibuf.c > create mode 100644 src/box/box_ibuf.h I would name it just src/box/ibuf.[ch]. > +void > +box_ibuf_read_range(box_ibuf_t *ibuf, char ***rpos, char ***wpos) > +{ > + if (!ibuf) > + return; I would assert on this. > + if (rpos) > + *rpos = &ibuf->rpos; > + if (wpos) > + *wpos = &ibuf->wpos; We use explicit == 0 / == NULL checks across our code. > index 000000000..235b87560 > --- /dev/null > +++ b/src/box/box_ibuf.h > @@ -0,0 +1,68 @@ > +#pragma once > +/* > <...> > + > +#include <stddef.h> It defines NULL and size_t as well as stdlib.h, but you include stdlib.h in the .c file, so I would use the same header in both. Not sure which one should be preferred. > + > +#include <trivia/util.h> > +#include <small/ibuf.h> How about just define an opaque structure? | struct ibuf; But include small/ibuf.h explicitly in the .c file? > +static int > +test_box_ibuf(lua_State *L) > +{ > + (void)L; There is usage of L at end of the function. > + struct slab_cache *slabc = cord_slab_cache(); > + assert(slabc != NULL); > + box_ibuf_t ibuf; > + > + ibuf_create(&ibuf, slabc, 16320); > + assert(ibuf_used(&ibuf) == 0); > + box_ibuf_reserve(&ibuf, 65536); > + char **rpos; > + char **wpos; > + box_ibuf_read_range(&ibuf, &rpos, &wpos); > + > + void *ptr = ibuf_alloc(&ibuf, 10); > + assert(ptr != NULL); > + > + assert(ibuf_used(&ibuf) == 10); > + assert((*wpos - *rpos) == 10); Now box_ibuf_read_range() should give the updated wpos. > + > + ptr = ibuf_alloc(&ibuf, 10000); > + assert(ptr); > + assert(ibuf_used(&ibuf) == 10010); > + assert((*wpos - *rpos) == 10010); Same here. > + > + ibuf_reset(&ibuf); > + assert(ibuf_used(&ibuf) == 0); I would test box_ibuf_write_range() as well. The test per se is more like how internal ibuf operations are interoperate with the public API rathen than a unit test of the public API. However it is okay, it'll also do the work and I would not bother much now.
next prev parent reply other threads:[~2020-10-13 18:21 UTC|newest] Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-10-12 0:44 [Tarantool-patches] [PATCH 2.X v3 0/3] module api: extend for external merger Lua module Timur Safin 2020-10-12 0:44 ` [Tarantool-patches] [PATCH 2.X v3 1/3] module api: export box_tuple_validate Timur Safin 2020-10-13 0:14 ` Alexander Turenko 2020-10-13 0:35 ` Timur Safin 2020-10-12 0:44 ` [Tarantool-patches] [PATCH 2.X v3 2/3] module api: export box_key_def_dup Timur Safin 2020-10-13 0:46 ` Alexander Turenko 2020-10-12 0:44 ` [Tarantool-patches] [PATCH 2.X v3 3/3] module api: luaL_checkibuf Timur Safin 2020-10-13 11:47 ` Alexander Turenko 2020-10-13 19:26 ` Igor Munkin 2020-10-13 16:30 ` [Tarantool-patches] [PATCH 2.X v3] module api: box_ibuf_* wrappers Timur Safin 2020-10-13 18:21 ` Alexander Turenko [this message] 2020-10-13 19:02 ` Timur Safin 2020-10-13 19:58 ` Alexander Turenko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20201013182145.stzjxwv3zmodm6sd@tkn_work_nb \ --to=alexander.turenko@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=tsafin@tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 2.X v3] module api: box_ibuf_* wrappers' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox