reftable: stop using `struct strbuf`
cover reftable: stop using `struct strbuf`
Hi,
this is the second patch series on my quest to make the reftable library become a standalone library again that can be used by libgit2 without pulling in all kinds of dependencies from the Git codebase. This part makes us lose the dependency on struct strbuf, which is done due to three reasons:
- To make us independent of libgit.a.
- To ensure that we use the pluggable allocators that users can set up
via `reftable_set_alloc()`.
- To make it possible to handle memory allocation failures.
While this leads to some duplication, we're only talking about ~70 lines of code.
Thanks!
Patrick
Patrick Steinhardt (10):
reftable: stop using strbuf_addbuf()
reftable: stop using strbuf_addf()
reftable/basics: provide new reftable_buf interface
reftable: convert from strbuf to reftable_buf reftable/blocksource: adapt interface name
t/unit-tests: check for reftable_buf allocation errors reftable/stack: adapt format_name() to handle allocation failures reftable/record: adapt reftable_record_key() to handle allocation
failures
reftable/stack: adapt stack_filename() to handle allocation failures
reftable: handle trivial reftable_buf errors
reftable/basics.c | 75 +++++++++- reftable/basics.h | 19 ++- reftable/block.c | 61 +++++--- reftable/block.h | 14 +- reftable/blocksource.c | 30 ++-- reftable/blocksource.h | 5 +- reftable/iter.c | 9 +- reftable/iter.h | 8 +- reftable/reader.c | 27 ++-- reftable/record.c | 114 ++++++++------ reftable/record.h | 21 +-- reftable/stack.c | 221 ++++++++++++++++++---------- reftable/system.h | 1 - reftable/writer.c | 102 ++++++++----- reftable/writer.h | 2 +- t/unit-tests/lib-reftable.c | 4 +- t/unit-tests/lib-reftable.h | 7 +- t/unit-tests/t-reftable-basics.c | 16 +- t/unit-tests/t-reftable-block.c | 53 +++---- t/unit-tests/t-reftable-merged.c | 32 ++-- t/unit-tests/t-reftable-reader.c | 12 +- t/unit-tests/t-reftable-readwrite.c | 134 +++++++++-------- t/unit-tests/t-reftable-record.c | 74 +++++----- t/unit-tests/t-reftable-stack.c | 96 ++++++------ 24 files changed, 685 insertions(+), 452 deletions(-)
Signature · 1 line
2.47.0.dirty
PATCH 1/10 reftable: stop using `strbuf_addbuf()`
We're about to introduce our own reftable_buf type to replace strbuf. Get rid of the seldomly-used strbuf_addbuf() function such that we have to reimplement one less function.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/block.c | 2 +- reftable/record.c | 6 +++--- reftable/writer.c | 7 ++++--- 3 files changed, 8 insertions(+), 7 deletions(-)
reftable/block.c +1 -1
@@ -60,7 +60,7 @@ static int block_writer_register_restart(struct block_writer *w, int n, w->next += n;strbuf_reset (&w->last_key); -strbuf_addbuf (&w->last_key, key); +strbuf_add (&w->last_key, key->buf, key->len); w->entries++;return 0 ; }
reftable/record.c +3 -3
@@ -1031,7 +1031,7 @@ static void reftable_index_record_key(const void *r, struct strbuf *dest) {const struct reftable_index_record *rec = r;strbuf_reset (dest); -strbuf_addbuf (dest, &rec->last_key); +strbuf_add (dest, rec->last_key.buf, rec->last_key.len); } static int reftable_index_record_copy_from(void *rec, const void *src_rec,
@@ -1041,7 +1041,7 @@ static int reftable_index_record_copy_from(void *rec, const void *src_rec,const struct reftable_index_record *src = src_rec;strbuf_reset (&dst->last_key); -strbuf_addbuf (&dst->last_key, &src->last_key); +strbuf_add (&dst->last_key, src->last_key.buf, src->last_key.len); dst->offset = src->offset;return 0 ;
@@ -1085,7 +1085,7 @@ static int reftable_index_record_decode(void *rec, struct strbuf key,int n =0 ;strbuf_reset (&r->last_key); -strbuf_addbuf (&r->last_key, &key); +strbuf_add (&r->last_key, key.buf, key.len); n =get_var_int (&r->offset, &in);if (n <0 )
reftable/writer.c +4 -3
@@ -225,7 +225,7 @@ static int writer_index_hash(struct reftable_writer *w, struct strbuf *hash) *key = empty;strbuf_reset (&key->hash); -strbuf_addbuf (&key->hash, hash); +strbuf_add (&key->hash, hash->buf, hash->len);tree_insert (&w->obj_index_tree, key, &obj_index_tree_node_compare); }else {
@@ -256,7 +256,7 @@ static int writer_add_record(struct reftable_writer *w, }strbuf_reset (&w->last_key); -strbuf_addbuf (&w->last_key, &key); +strbuf_add (&w->last_key, key.buf, key.len);if (!w->block_writer) { err =writer_reinit_block_writer (w,reftable_record_type (rec));if (err <0 )
@@ -778,7 +778,8 @@ static int writer_flush_nonempty_block(struct reftable_writer *w) index_record.offset = w->next;strbuf_reset (&index_record.last_key); -strbuf_addbuf (&index_record.last_key, &w->block_writer->last_key); +strbuf_add (&index_record.last_key, w->block_writer->last_key.buf, + w->block_writer->last_key.len); w->index[w->index_len] = index_record; w->index_len++;
Signature · 1 line
2.47.0.dirty
PATCH 2/10 reftable: stop using `strbuf_addf()`
In reply to Patrick SteinhardtWe're about to introduce our own reftable_buf type to replace strbuf. Get rid of the seldomly-used strbuf_addf() function such that we have to reimplement one less function.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/stack.c | 18 ++++++++----- t/unit-tests/t-reftable-block.c | 7 +++-- t/unit-tests/t-reftable-readwrite.c | 20 +++++++------- t/unit-tests/t-reftable-stack.c | 42 ++++++++++++++++------------- 4 files changed, 50 insertions(+), 37 deletions(-)
reftable/stack.c +12 -6
@@ -1387,12 +1387,18 @@ static int stack_compact_range(struct reftable_stack *st,* have just written. In case the compacted table became empty we * simply skip writing it. */ -for (i =0 ; i < first_to_replace; i++) -strbuf_addf (&tables_list_buf,"%s\n" , names[i]); -if (!is_empty_table) -strbuf_addf (&tables_list_buf,"%s\n" , new_table_name.buf); -for (i = last_to_replace +1 ; names[i]; i++) -strbuf_addf (&tables_list_buf,"%s\n" , names[i]); +for (i =0 ; i < first_to_replace; i++) { +strbuf_addstr (&tables_list_buf, names[i]); +strbuf_addstr (&tables_list_buf,"\n" ); + } +if (!is_empty_table) { +strbuf_addstr (&tables_list_buf, new_table_name.buf); +strbuf_addstr (&tables_list_buf,"\n" ); + } +for (i = last_to_replace +1 ; names[i]; i++) { +strbuf_addstr (&tables_list_buf, names[i]); +strbuf_addstr (&tables_list_buf,"\n" ); + } err =write_in_full (get_lock_file_fd (&tables_list_lock), tables_list_buf.buf, tables_list_buf.len);
t/unit-tests/t-reftable-block.c +5 -2
@@ -308,10 +308,13 @@ static void t_index_block_read_write(void)check (!ret);for (i =0 ; i <N ; i++) { -strbuf_init (&recs[i].u.idx.last_key,9 ); +char buf[128 ]; + +snprintf (buf,sizeof (buf),"branch%02" PRIuMAX, (uintmax_t )i); +strbuf_init (&recs[i].u.idx.last_key,9 ); recs[i].type =BLOCK_TYPE_INDEX ; -strbuf_addf (&recs[i].u.idx.last_key,"branch%02" PRIuMAX, (uintmax_t )i); +strbuf_addstr (&recs[i].u.idx.last_key, buf); recs[i].u.idx.offset = i; ret =block_writer_add (&bw, &recs[i]);
t/unit-tests/t-reftable-readwrite.c +9 -11
@@ -753,12 +753,13 @@ static void t_write_multiple_indices(void)struct reftable_write_options opts = { .block_size =100 , }; -struct strbuf writer_buf =STRBUF_INIT , buf =STRBUF_INIT ; +struct strbuf writer_buf =STRBUF_INIT ;struct reftable_block_source source = {0 };struct reftable_iterator it = {0 };const struct reftable_stats *stats;struct reftable_writer *writer;struct reftable_reader *reader; +char buf[128 ];int err, i; writer =t_reftable_strbuf_writer (&writer_buf, &opts);
@@ -770,9 +771,8 @@ static void t_write_multiple_indices(void) .value.val1 = {i}, }; -strbuf_reset (&buf); -strbuf_addf (&buf,"refs/heads/%04d" , i); - ref.refname = buf.buf, +snprintf (buf,sizeof (buf),"refs/heads/%04d" , i); + ref.refname = buf; err =reftable_writer_add_ref (writer, &ref);check (!err);
@@ -788,9 +788,8 @@ static void t_write_multiple_indices(void) }, }; -strbuf_reset (&buf); -strbuf_addf (&buf,"refs/heads/%04d" , i); - log.refname = buf.buf, +snprintf (buf,sizeof (buf),"refs/heads/%04d" , i); + log.refname = buf; err =reftable_writer_add_log (writer, &log);check (!err);
@@ -824,7 +823,6 @@ static void t_write_multiple_indices(void)reftable_writer_free (writer);reftable_reader_decref (reader);strbuf_release (&writer_buf); -strbuf_release (&buf); }static void t_write_multi_level_index (void )
@@ -848,10 +846,10 @@ static void t_write_multi_level_index(void) .value_type = REFTABLE_REF_VAL1, .value.val1 = {i}, }; +char buf[128 ]; -strbuf_reset (&buf); -strbuf_addf (&buf,"refs/heads/%03" PRIuMAX, (uintmax_t )i); - ref.refname = buf.buf, +snprintf (buf,sizeof (buf),"refs/heads/%03" PRIuMAX, (uintmax_t )i); + ref.refname = buf; err =reftable_writer_add_ref (writer, &ref);check (!err);
t/unit-tests/t-reftable-stack.c +24 -18
110 lines not on this page · raw message
Signature · 1 line
2.47.0.dirty
PATCH 3/10 reftable/basics: provide new `reftable_buf` interface
In reply to Patrick SteinhardtImplement a new reftable_buf interface that will replace Git's own strbuf interface. This is done due to three reasons:
- The strbuf interfaces do not handle memory allocation failures and instead causes us to die. This is okay in the context of Git, but is not context of the reftable library, which is supposed to be usable by third-party applications.
- The strbuf interface is quite deeply tied into Git, which makes it hard to use the reftable library as a standalone library. Any dependent would have to carefully extract the relevant parts of it to make things work, which is not all that sensible.
- The strbuf interface does not use the pluggable allocators that can be set up via refatble_set_alloc().
So we have good reasons to use our own type, and the implementation is rather trivial. Implement our own type. Conversion of the reftable library will be handled in subsequent commits.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/basics.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++ reftable/basics.h | 16 +++++++++++ 2 files changed, 89 insertions(+)
reftable/basics.c +73 -0
@@ -69,6 +69,79 @@ void reftable_set_alloc(void *(*malloc)(size_t), reftable_free_ptr = free; } +void reftable_buf_init (struct reftable_buf *buf) +{ +struct reftable_buf empty =REFTABLE_BUF_INIT ; + *buf = empty; +} + +void reftable_buf_release (struct reftable_buf *buf) +{ +reftable_free (buf->buf); +reftable_buf_init (buf); +} + +void reftable_buf_reset (struct reftable_buf *buf) +{ +if (buf->alloc) { + buf->len =0 ; + buf->buf[0 ] ='\0' ; + } +} + +int reftable_buf_setlen (struct reftable_buf *buf,size_t len) +{ +if (len > buf->len) +return -1 ; +if (len == buf->len) +return 0 ; + buf->buf[len] ='\0' ; + buf->len = len; +return 0 ; +} + +int reftable_buf_cmp (const struct reftable_buf *a,const struct reftable_buf *b) +{ +size_t len = a->len < b->len ? a->len : b->len; +if (len) { +int cmp =memcmp (a->buf, b->buf, len); +if (cmp) +return cmp; + } +return a->len < b->len ?-1 : a->len != b->len; +} + +int reftable_buf_add (struct reftable_buf *buf,const void *data,size_t len) +{ +size_t newlen = buf->len + len; + +if (newlen +1 > buf->alloc) { +char *reallocated = buf->buf; +REFTABLE_ALLOC_GROW (reallocated, newlen +1 , buf->alloc); +if (!reallocated) +return -1 ; + buf->buf = reallocated; + } + +memcpy (buf->buf + buf->len, data, len); + buf->buf[newlen] ='\0' ; + buf->len = newlen; + +return 0 ; +} + +int reftable_buf_addstr (struct reftable_buf *buf,const char *s) +{ +return reftable_buf_add (buf, s,strlen (s)); +} + +char *reftable_buf_detach (struct reftable_buf *buf) +{ +char *result = buf->buf; +reftable_buf_init (buf); +return result; +} +void put_be24 (uint8_t *out,uint32_t i) { out[0 ] = (uint8_t )((i >>16 ) &0xff );
reftable/basics.h +16 -0
@@ -16,6 +16,22 @@ license that can be found in the LICENSE file or at#include "system.h" #include "reftable-basics.h" +struct reftable_buf { +size_t alloc; +size_t len; +char *buf; +}; +#define REFTABLE_BUF_INIT { 0 } + +void reftable_buf_init (struct reftable_buf *buf); +void reftable_buf_release (struct reftable_buf *buf); +void reftable_buf_reset (struct reftable_buf *buf); +int reftable_buf_setlen (struct reftable_buf *buf,size_t len); +int reftable_buf_cmp (const struct reftable_buf *a,const struct reftable_buf *b); +int reftable_buf_add (struct reftable_buf *buf,const void *data,size_t len); +int reftable_buf_addstr (struct reftable_buf *buf,const char *s); +char *reftable_buf_detach (struct reftable_buf *buf); +/* Bigendian en/decoding of integers */ void put_be24 (uint8_t *out,uint32_t i);
Signature · 1 line
2.47.0.dirty
PATCH 4/10 reftable: convert from `strbuf` to `reftable_buf`
In reply to Patrick SteinhardtConvert the reftable library to use the reftable_buf interface instead of the strbuf interface. This is a mechanical change via sed(1) and does not yet handle allocation failures. These will be addressed in subsequent commits.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/basics.c | 2 +- reftable/basics.h | 3 +- reftable/block.c | 34 ++++---- reftable/block.h | 14 ++-- reftable/blocksource.c | 6 +- reftable/blocksource.h | 3 +- reftable/iter.c | 6 +- reftable/iter.h | 8 +- reftable/reader.c | 16 ++-- reftable/record.c | 80 +++++++++---------- reftable/record.h | 21 ++--- reftable/stack.c | 120 ++++++++++++++-------------- reftable/system.h | 1 - reftable/writer.c | 66 +++++++-------- reftable/writer.h | 2 +- t/unit-tests/lib-reftable.c | 4 +- t/unit-tests/lib-reftable.h | 7 +- t/unit-tests/t-reftable-basics.c | 16 ++-- t/unit-tests/t-reftable-block.c | 42 +++++----- t/unit-tests/t-reftable-merged.c | 26 +++--- t/unit-tests/t-reftable-reader.c | 8 +- t/unit-tests/t-reftable-readwrite.c | 92 ++++++++++----------- t/unit-tests/t-reftable-record.c | 74 ++++++++--------- t/unit-tests/t-reftable-stack.c | 90 ++++++++++----------- 24 files changed, 371 insertions(+), 370 deletions(-)
reftable/basics.c +1 -1
@@ -259,7 +259,7 @@ int names_equal(const char **a, const char **b)return a[i] == b[i]; } -int common_prefix_size (struct strbuf *a,struct strbuf *b) +int common_prefix_size (struct reftable_buf *a,struct reftable_buf *b) {int p =0 ;for (; p < a->len && p < b->len; p++) {
reftable/basics.h +1 -2
@@ -104,8 +104,7 @@ char *reftable_strdup(const char *str);#endif /* Find the longest shared prefix size of `a` and `b` */ -struct strbuf ; -int common_prefix_size (struct strbuf *a,struct strbuf *b); +int common_prefix_size (struct reftable_buf *a,struct reftable_buf *b);int hash_size (uint32_t id);
reftable/block.c +17 -17
@@ -38,7 +38,7 @@ int footer_size(int version) }static int block_writer_register_restart (struct block_writer *w,int n, -int is_restart,struct strbuf *key) +int is_restart,struct reftable_buf *key) {int rlen = w->restart_len;if (rlen >=MAX_RESTARTS ) {
@@ -59,8 +59,8 @@ static int block_writer_register_restart(struct block_writer *w, int n, w->next += n; -strbuf_reset (&w->last_key); -strbuf_add (&w->last_key, key->buf, key->len); +reftable_buf_reset (&w->last_key); +reftable_buf_add (&w->last_key, key->buf, key->len); w->entries++;return 0 ; }
@@ -98,8 +98,8 @@ uint8_t block_writer_type(struct block_writer *bw)empty key. */ int block_writer_add (struct block_writer *w,struct reftable_record *rec) { -struct strbuf empty =STRBUF_INIT ; -struct strbuf last = +struct reftable_buf empty =REFTABLE_BUF_INIT ; +struct reftable_buf last = w->entries % w->restart_interval ==0 ? empty : w->last_key;struct string_view out = { .buf = w->buf + w->next,
@@ -109,7 +109,7 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)struct string_view start = out;int is_restart =0 ; -struct strbuf key =STRBUF_INIT ; +struct reftable_buf key =REFTABLE_BUF_INIT ;int n =0 ;int err =-1 ;
@@ -133,7 +133,7 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec) err =block_writer_register_restart (w, start.len - out.len, is_restart, &key); done: -strbuf_release (&key); +reftable_buf_release (&key);return err; }
@@ -325,7 +325,7 @@ uint8_t block_reader_type(const struct block_reader *r)return r->block.data[r->header_off]; } -int block_reader_first_key (const struct block_reader *br,struct strbuf *key) +int block_reader_first_key (const struct block_reader *br,struct reftable_buf *key) {int off = br->header_off +4 , n;struct string_view in = {
@@ -334,7 +334,7 @@ int block_reader_first_key(const struct block_reader *br, struct strbuf *key) };uint8_t extra =0 ; -strbuf_reset (key); +reftable_buf_reset (key); n =reftable_decode_key (key, &extra, in);if (n <0 )
@@ -355,13 +355,13 @@ void block_iter_seek_start(struct block_iter *it, const struct block_reader *br) it->block = br->block.data; it->block_len = br->block_len; it->hash_size = br->hash_size; -strbuf_reset (&it->last_key); +reftable_buf_reset (&it->last_key); it->next_off = br->header_off +4 ; }struct restart_needle_less_args {int error; -struct strbuf needle; +struct reftable_buf needle;const struct block_reader *reader; };
@@ -433,7 +433,7 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec)void block_iter_reset (struct block_iter *it) { -strbuf_reset (&it->last_key); +reftable_buf_reset (&it->last_key); it->next_off =0 ; it->block =NULL ; it->block_len =0 ;
@@ -442,12 +442,12 @@ void block_iter_reset(struct block_iter *it)void block_iter_close (struct block_iter *it) { -strbuf_release (&it->last_key); -strbuf_release (&it->scratch); +reftable_buf_release (&it->last_key); +reftable_buf_release (&it->scratch); }int block_iter_seek_key (struct block_iter *it,const struct block_reader *br, -struct strbuf *want) +struct reftable_buf *want) {struct restart_needle_less_args args = { .needle = *want,
@@ -537,7 +537,7 @@ int block_iter_seek_key(struct block_iter *it, const struct block_reader *br,* with themselves. */ reftable_record_key (&rec, &it->last_key); -if (strbuf_cmp (&it->last_key, want) >=0 ) { +if (reftable_buf_cmp (&it->last_key, want) >=0 ) { it->next_off = prev_off; goto done; }
@@ -554,7 +554,7 @@ void block_writer_release(struct block_writer *bw)REFTABLE_FREE_AND_NULL (bw->zstream);REFTABLE_FREE_AND_NULL (bw->restarts);REFTABLE_FREE_AND_NULL (bw->compressed); -strbuf_release (&bw->last_key); +reftable_buf_release (&bw->last_key);/* the block is not owned. */ }
reftable/block.h +7 -7
@@ -38,7 +38,7 @@ struct block_writer {uint32_t restart_len;uint32_t restart_cap; -struct strbuf last_key; +struct reftable_buf last_key;int entries; };
@@ -98,7 +98,7 @@ void block_reader_release(struct block_reader *br);uint8_t block_reader_type (const struct block_reader *r);/* Decodes the first key in the block */ -int block_reader_first_key (const struct block_reader *br,struct strbuf *key); +int block_reader_first_key (const struct block_reader *br,struct reftable_buf *key);/* Iterate over entries in a block */ struct block_iter {
@@ -109,13 +109,13 @@ struct block_iter {int hash_size;/* key for last entry we read. */ -struct strbuf last_key; -struct strbuf scratch; +struct reftable_buf last_key; +struct reftable_buf scratch; };#define BLOCK_ITER_INIT { \ - .last_key = STRBUF_INIT, \ - .scratch = STRBUF_INIT, \ + .last_key = REFTABLE_BUF_INIT, \ + .scratch = REFTABLE_BUF_INIT, \ }/* Position `it` at start of the block */
@@ -123,7 +123,7 @@ void block_iter_seek_start(struct block_iter *it, const struct block_reader *br)/* Position `it` to the `want` key in the block */ int block_iter_seek_key (struct block_iter *it,const struct block_reader *br, -struct strbuf *want); +struct reftable_buf *want);/* return < 0 for error, 0 for OK, > 0 for EOF. */ int block_iter_next (struct block_iter *it,struct reftable_record *rec);
reftable/blocksource.c +3 -3
@@ -27,7 +27,7 @@ static void strbuf_close(void *b UNUSED)static int strbuf_read_block (void *v,struct reftable_block *dest,uint64_t off,uint32_t size) { -struct strbuf *b = v; +struct reftable_buf *b = v;assert (off + size <= b->len);REFTABLE_CALLOC_ARRAY (dest->data, size);if (!dest->data)
@@ -39,7 +39,7 @@ static int strbuf_read_block(void *v, struct reftable_block *dest, uint64_t off,static uint64_t strbuf_size (void *b) { -return ((struct strbuf *)b)->len; +return ((struct reftable_buf *)b)->len; }static struct reftable_block_source_vtable strbuf_vtable = {
@@ -50,7 +50,7 @@ static struct reftable_block_source_vtable strbuf_vtable = { };void block_source_from_strbuf (struct reftable_block_source *bs, -struct strbuf *buf) +struct reftable_buf *buf) {assert (!bs->ops); bs->ops = &strbuf_vtable;
reftable/blocksource.h +2 -1
@@ -12,9 +12,10 @@ license that can be found in the LICENSE file or at#include "system.h" struct reftable_block_source ; +struct reftable_buf ;/* Create an in-memory block source for reading reftables */ void block_source_from_strbuf (struct reftable_block_source *bs, -struct strbuf *buf); +struct reftable_buf *buf);#endif
reftable/iter.c +3 -3
@@ -55,7 +55,7 @@ void iterator_set_empty(struct reftable_iterator *it)static void filtering_ref_iterator_close (void *iter_arg) {struct filtering_ref_iterator *fri = iter_arg; -strbuf_release (&fri->oid); +reftable_buf_release (&fri->oid);reftable_iterator_destroy (&fri->it); }
@@ -115,7 +115,7 @@ static void indexed_table_ref_iter_close(void *p)block_iter_close (&it->cur);reftable_block_done (&it->block_reader.block);reftable_free (it->offsets); -strbuf_release (&it->oid); +reftable_buf_release (&it->oid); }static int indexed_table_ref_iter_next_block (struct indexed_table_ref_iter *it)
@@ -197,7 +197,7 @@ int indexed_table_ref_iter_new(struct indexed_table_ref_iter **dest, *itr = empty; itr->r = r; -strbuf_add (&itr->oid, oid, oid_len); +reftable_buf_add (&itr->oid, oid, oid_len); itr->offsets = offsets; itr->offset_len = offset_len;
reftable/iter.h +4 -4
@@ -44,12 +44,12 @@ void iterator_set_empty(struct reftable_iterator *it);/* iterator that produces only ref records that point to `oid` */ struct filtering_ref_iterator { -struct strbuf oid; +struct reftable_buf oid;struct reftable_iterator it; };#define FILTERING_REF_ITERATOR_INIT \ { \ - .oid = STRBUF_INIT \ + .oid = REFTABLE_BUF_INIT \ } void iterator_from_filtering_ref_iterator(struct reftable_iterator *,
@@ -60,7 +60,7 @@ void iterator_from_filtering_ref_iterator(struct reftable_iterator *,*/ struct indexed_table_ref_iter {struct reftable_reader *r; -struct strbuf oid; +struct reftable_buf oid;/* mutable */ uint64_t *offsets;
@@ -75,7 +75,7 @@ struct indexed_table_ref_iter {#define INDEXED_TABLE_REF_ITER_INIT { \ .cur = BLOCK_ITER_INIT, \ - .oid = STRBUF_INIT, \ + .oid = REFTABLE_BUF_INIT, \ } void iterator_from_indexed_table_ref_iter(struct reftable_iterator *it,
reftable/reader.c +8 -8
@@ -350,8 +350,8 @@ static int table_iter_seek_start(struct table_iter *ti, uint8_t typ, int index)static int table_iter_seek_linear (struct table_iter *ti,struct reftable_record *want) { -struct strbuf want_key =STRBUF_INIT ; -struct strbuf got_key =STRBUF_INIT ; +struct reftable_buf want_key =REFTABLE_BUF_INIT ; +struct reftable_buf got_key =REFTABLE_BUF_INIT ;struct reftable_record rec;int err;
@@ -401,7 +401,7 @@ static int table_iter_seek_linear(struct table_iter *ti,if (err <0 ) goto done; -if (strbuf_cmp (&got_key, &want_key) >0 ) { +if (reftable_buf_cmp (&got_key, &want_key) >0 ) {table_iter_block_done (&next);break ; }
@@ -422,8 +422,8 @@ static int table_iter_seek_linear(struct table_iter *ti, done:reftable_record_release (&rec); -strbuf_release (&want_key); -strbuf_release (&got_key); +reftable_buf_release (&want_key); +reftable_buf_release (&got_key);return err; }
@@ -431,11 +431,11 @@ static int table_iter_seek_indexed(struct table_iter *ti, struct reftable_record *rec) {struct reftable_record want_index = { - .type =BLOCK_TYPE_INDEX , .u.idx = { .last_key =STRBUF_INIT } + .type =BLOCK_TYPE_INDEX , .u.idx = { .last_key =REFTABLE_BUF_INIT } };struct reftable_record index_result = { .type =BLOCK_TYPE_INDEX , - .u.idx = { .last_key =STRBUF_INIT }, + .u.idx = { .last_key =REFTABLE_BUF_INIT }, };int err;
@@ -765,7 +765,7 @@ static int reftable_reader_refs_for_unindexed(struct reftable_reader *r, } *filter = empty; -strbuf_add (&filter->oid, oid, oid_len); +reftable_buf_add (&filter->oid, oid, oid_len);iterator_from_table_iter (&filter->it, ti);iterator_from_filtering_ref_iterator (it, filter);
reftable/record.c +40 -40
235 lines not on this page · raw message
reftable/record.h +11 -10
@@ -9,6 +9,7 @@ license that can be found in the LICENSE file or at#ifndef RECORD_H #define RECORD_H +#include "basics.h" #include "system.h" #include <stdint.h>
@@ -38,8 +39,8 @@ int put_var_int(struct string_view *dest, uint64_t val);/* Methods for records. */ struct reftable_record_vtable { -/* encode the key of to a uint8_t strbuf. */ -void (*key)(const void *rec,struct strbuf *dest); +/* encode the key of to a uint8_t reftable_buf. */ +void (*key)(const void *rec,struct reftable_buf *dest);/* The record type of ('r' for ref). */ uint8_t type;
@@ -54,9 +55,9 @@ struct reftable_record_vtable {int (*encode)(const void *rec,struct string_view dest,int hash_size);/* decode data from `src` into the record. */ -int (*decode)(void *rec,struct strbuf key,uint8_t extra, +int (*decode)(void *rec,struct reftable_buf key,uint8_t extra,struct string_view src,int hash_size, -struct strbuf *scratch); +struct reftable_buf *scratch);/* deallocate and null the record. */ void (*release)(void *rec);
@@ -83,7 +84,7 @@ int reftable_is_block_type(uint8_t typ);/* Encode `key` into `dest`. Sets `is_restart` to indicate a restart. Returns * number of bytes written. */ int reftable_encode_key (int *is_restart,struct string_view dest, -struct strbuf prev_key,struct strbuf key, +struct reftable_buf prev_key,struct reftable_buf key,uint8_t extra);/* Decode a record's key lengths. */
@@ -96,13 +97,13 @@ int reftable_decode_keylen(struct string_view in,* Decode into `last_key` and `extra` from `in`. `last_key` is expected to * contain the decoded key of the preceding record, if any. */ -int reftable_decode_key (struct strbuf *last_key,uint8_t *extra, +int reftable_decode_key (struct reftable_buf *last_key,uint8_t *extra,struct string_view in);/* reftable_index_record are used internally to speed up lookups. */ struct reftable_index_record {uint64_t offset;/* Offset of block */ -struct strbuf last_key;/* Last key of the block. */ +struct reftable_buf last_key;/* Last key of the block. */ };/* reftable_obj_record stores an object ID => ref mapping. */
@@ -136,15 +137,15 @@ void reftable_record_init(struct reftable_record *rec, uint8_t typ);/* see struct record_vtable */ int reftable_record_cmp (struct reftable_record *a,struct reftable_record *b);int reftable_record_equal (struct reftable_record *a,struct reftable_record *b,int hash_size); -void reftable_record_key (struct reftable_record *rec,struct strbuf *dest); +void reftable_record_key (struct reftable_record *rec,struct reftable_buf *dest);int reftable_record_copy_from (struct reftable_record *rec,struct reftable_record *src,int hash_size);uint8_t reftable_record_val_type (struct reftable_record *rec);int reftable_record_encode (struct reftable_record *rec,struct string_view dest,int hash_size); -int reftable_record_decode (struct reftable_record *rec,struct strbuf key, +int reftable_record_decode (struct reftable_record *rec,struct reftable_buf key,uint8_t extra,struct string_view src, -int hash_size,struct strbuf *scratch); +int hash_size,struct reftable_buf *scratch);int reftable_record_is_deletion (struct reftable_record *rec);static inline uint8_t reftable_record_type (struct reftable_record *rec)
reftable/stack.c +60 -60
305 lines not on this page · raw message
reftable/system.h +0 -1
@@ -13,7 +13,6 @@ license that can be found in the LICENSE file or at#include "git-compat-util.h" #include "lockfile.h" -#include "strbuf.h" #include "tempfile.h" #include "hash.h" /* hash ID, sizes.*/ #include "dir.h" /* remove_dir_recursively, for tests.*/
reftable/writer.c +33 -33
219 lines not on this page · raw message
reftable/writer.h +1 -1
@@ -19,7 +19,7 @@ struct reftable_writer {int (*flush)(void *);void *write_arg;int pending_padding; -struct strbuf last_key; +struct reftable_buf last_key;/* offset of next block to write. */ uint64_t next;
t/unit-tests/lib-reftable.c +2 -2
@@ -19,7 +19,7 @@ static int strbuf_writer_flush(void *arg UNUSED)return 0 ; } -struct reftable_writer *t_reftable_strbuf_writer (struct strbuf *buf, +struct reftable_writer *t_reftable_strbuf_writer (struct reftable_buf *buf,struct reftable_write_options *opts) {struct reftable_writer *writer;
@@ -29,7 +29,7 @@ struct reftable_writer *t_reftable_strbuf_writer(struct strbuf *buf,return writer; } -void t_reftable_write_to_buf(struct strbuf *buf, +void t_reftable_write_to_buf(struct reftable_buf *buf, struct reftable_ref_record *refs, size_t nrefs, struct reftable_log_record *logs,
t/unit-tests/lib-reftable.h +4 -3
@@ -2,15 +2,16 @@#define LIB_REFTABLE_H #include "git-compat-util.h" -#include "strbuf.h" #include "reftable/reftable-writer.h" +struct reftable_buf ; +void t_reftable_set_hash (uint8_t *p,int i,uint32_t id); -struct reftable_writer *t_reftable_strbuf_writer (struct strbuf *buf, +struct reftable_writer *t_reftable_strbuf_writer (struct reftable_buf *buf,struct reftable_write_options *opts); -void t_reftable_write_to_buf(struct strbuf *buf, +void t_reftable_write_to_buf(struct reftable_buf *buf, struct reftable_ref_record *refs, size_t nrecords, struct reftable_log_record *logs,
t/unit-tests/t-reftable-basics.c +8 -8
@@ -99,8 +99,8 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED) } if_test ("common_prefix_size works") { -struct strbuf a =STRBUF_INIT ; -struct strbuf b =STRBUF_INIT ; +struct reftable_buf a =REFTABLE_BUF_INIT ; +struct reftable_buf b =REFTABLE_BUF_INIT ;struct {const char *a, *b;int want;
@@ -113,14 +113,14 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED) };for (size_t i =0 ; i <ARRAY_SIZE (cases); i++) { -strbuf_addstr (&a, cases[i].a); -strbuf_addstr (&b, cases[i].b); +reftable_buf_addstr (&a, cases[i].a); +reftable_buf_addstr (&b, cases[i].b); check_int(common_prefix_size(&a, &b), ==, cases[i].want); -strbuf_reset (&a); -strbuf_reset (&b); +reftable_buf_reset (&a); +reftable_buf_reset (&b); } -strbuf_release (&a); -strbuf_release (&b); +reftable_buf_release (&a); +reftable_buf_release (&b); } if_test ("put_be24 and get_be24 work") {
t/unit-tests/t-reftable-block.c +21 -21
129 lines not on this page · raw message
t/unit-tests/t-reftable-merged.c +13 -13
@@ -20,7 +20,7 @@ static struct reftable_merged_table * merged_table_from_records(struct reftable_ref_record **refs, struct reftable_block_source **source, struct reftable_reader ***readers, const size_t *sizes, - struct strbuf *buf, const size_t n) + struct reftable_buf *buf, const size_t n) { struct reftable_merged_table *mt = NULL; struct reftable_write_options opts = {
@@ -75,7 +75,7 @@ static void t_merged_single_record(void)struct reftable_ref_record *refs[] = { r1, r2, r3 };size_t sizes[] = {ARRAY_SIZE (r1),ARRAY_SIZE (r2),ARRAY_SIZE (r3) }; -struct strbuf bufs[3 ] = {STRBUF_INIT ,STRBUF_INIT ,STRBUF_INIT }; +struct reftable_buf bufs[3 ] = {REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT };struct reftable_block_source *bs =NULL ;struct reftable_reader **readers =NULL ; struct reftable_merged_table *mt =
@@ -97,7 +97,7 @@ static void t_merged_single_record(void)readers_destroy (readers,3 );reftable_merged_table_free (mt);for (size_t i =0 ; i <ARRAY_SIZE (bufs); i++) -strbuf_release (&bufs[i]); +reftable_buf_release (&bufs[i]);reftable_free (bs); }
@@ -152,7 +152,7 @@ static void t_merged_refs(void)struct reftable_ref_record *refs[] = { r1, r2, r3 };size_t sizes[3 ] = {ARRAY_SIZE (r1),ARRAY_SIZE (r2),ARRAY_SIZE (r3) }; -struct strbuf bufs[3 ] = {STRBUF_INIT ,STRBUF_INIT ,STRBUF_INIT }; +struct reftable_buf bufs[3 ] = {REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT };struct reftable_block_source *bs =NULL ;struct reftable_reader **readers =NULL ; struct reftable_merged_table *mt =
@@ -192,7 +192,7 @@ static void t_merged_refs(void)reftable_free (out);for (i =0 ; i <3 ; i++) -strbuf_release (&bufs[i]); +reftable_buf_release (&bufs[i]);readers_destroy (readers,3 );reftable_merged_table_free (mt);reftable_free (bs);
@@ -234,8 +234,8 @@ static void t_merged_seek_multiple_times(void)size_t sizes[] = {ARRAY_SIZE (r1),ARRAY_SIZE (r2), }; -struct strbuf bufs[] = { -STRBUF_INIT ,STRBUF_INIT , +struct reftable_buf bufs[] = { +REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT , };struct reftable_block_source *sources =NULL ;struct reftable_reader **readers =NULL ;
@@ -265,7 +265,7 @@ static void t_merged_seek_multiple_times(void) }for (size_t i =0 ; i <ARRAY_SIZE (bufs); i++) -strbuf_release (&bufs[i]); +reftable_buf_release (&bufs[i]);readers_destroy (readers,ARRAY_SIZE (refs));reftable_ref_record_release (&rec);reftable_iterator_destroy (&it);
@@ -277,7 +277,7 @@ static struct reftable_merged_table * merged_table_from_log_records(struct reftable_log_record **logs, struct reftable_block_source **source, struct reftable_reader ***readers, const size_t *sizes, - struct strbuf *buf, const size_t n) + struct reftable_buf *buf, const size_t n) { struct reftable_merged_table *mt = NULL; struct reftable_write_options opts = {
@@ -361,7 +361,7 @@ static void t_merged_logs(void)struct reftable_log_record *logs[] = { r1, r2, r3 };size_t sizes[3 ] = {ARRAY_SIZE (r1),ARRAY_SIZE (r2),ARRAY_SIZE (r3) }; -struct strbuf bufs[3 ] = {STRBUF_INIT ,STRBUF_INIT ,STRBUF_INIT }; +struct reftable_buf bufs[3 ] = {REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT };struct reftable_block_source *bs =NULL ;struct reftable_reader **readers =NULL ; struct reftable_merged_table *mt = merged_table_from_log_records(
@@ -412,7 +412,7 @@ static void t_merged_logs(void)reftable_free (out);for (i =0 ; i <3 ; i++) -strbuf_release (&bufs[i]); +reftable_buf_release (&bufs[i]);readers_destroy (readers,3 );reftable_merged_table_free (mt);reftable_free (bs);
@@ -421,7 +421,7 @@ static void t_merged_logs(void)static void t_default_write_opts (void ) {struct reftable_write_options opts = {0 }; -struct strbuf buf =STRBUF_INIT ; +struct reftable_buf buf =REFTABLE_BUF_INIT ;struct reftable_writer *w =t_reftable_strbuf_writer (&buf, &opts);struct reftable_ref_record rec = { .refname = (char *)"master" ,
@@ -457,7 +457,7 @@ static void t_default_write_opts(void)reftable_reader_decref (rd);reftable_merged_table_free (merged); -strbuf_release (&buf); +reftable_buf_release (&buf); }
t/unit-tests/t-reftable-reader.c +4 -4
@@ -16,7 +16,7 @@ static int t_reader_seek_once(void)struct reftable_ref_record ref = {0 };struct reftable_iterator it = {0 };struct reftable_reader *reader; -struct strbuf buf =STRBUF_INIT ; +struct reftable_buf buf =REFTABLE_BUF_INIT ;int ret;t_reftable_write_to_buf (&buf, records,ARRAY_SIZE (records),NULL ,0 ,NULL );
@@ -40,7 +40,7 @@ static int t_reader_seek_once(void)reftable_ref_record_release (&ref);reftable_iterator_destroy (&it);reftable_reader_decref (reader); -strbuf_release (&buf); +reftable_buf_release (&buf);return 0 ; }
@@ -57,7 +57,7 @@ static int t_reader_reseek(void)struct reftable_ref_record ref = {0 };struct reftable_iterator it = {0 };struct reftable_reader *reader; -struct strbuf buf =STRBUF_INIT ; +struct reftable_buf buf =REFTABLE_BUF_INIT ;int ret;t_reftable_write_to_buf (&buf, records,ARRAY_SIZE (records),NULL ,0 ,NULL );
@@ -84,7 +84,7 @@ static int t_reader_reseek(void)reftable_ref_record_release (&ref);reftable_iterator_destroy (&it);reftable_reader_decref (reader); -strbuf_release (&buf); +reftable_buf_release (&buf);return 0 ; }
t/unit-tests/t-reftable-readwrite.c +46 -46
328 lines not on this page · raw message
t/unit-tests/t-reftable-record.c +37 -37
201 lines not on this page · raw message
t/unit-tests/t-reftable-stack.c +45 -45
214 lines not on this page · raw message
Signature · 1 line
2.47.0.dirty
PATCH 5/10 reftable/blocksource: adapt interface name
In reply to Patrick SteinhardtAdapt the name of the strbuf block source to no longer relate to this interface, but instead to the reftable_buf interface.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/blocksource.c | 26 +++++++++++++------------- reftable/blocksource.h | 4 ++-- t/unit-tests/t-reftable-block.c | 8 ++++---- t/unit-tests/t-reftable-merged.c | 6 +++--- t/unit-tests/t-reftable-reader.c | 4 ++-- t/unit-tests/t-reftable-readwrite.c | 24 ++++++++++++------------ 6 files changed, 36 insertions(+), 36 deletions(-)
reftable/blocksource.c +13 -13
@@ -13,19 +13,19 @@ license that can be found in the LICENSE file or at#include "reftable-blocksource.h" #include "reftable-error.h" -static void strbuf_return_block(void *b UNUSED, struct reftable_block *dest) +static void reftable_buf_return_block(void *b UNUSED, struct reftable_block *dest) {if (dest->len)memset (dest->data,0xff , dest->len);reftable_free (dest->data); } -static void strbuf_close(void *b UNUSED) +static void reftable_buf_close(void *b UNUSED) { } -static int strbuf_read_block (void *v,struct reftable_block *dest,uint64_t off, -uint32_t size) +static int reftable_buf_read_block (void *v,struct reftable_block *dest, +uint64_t off,uint32_t size) {struct reftable_buf *b = v;assert (off + size <= b->len);
@@ -37,23 +37,23 @@ static int strbuf_read_block(void *v, struct reftable_block *dest, uint64_t off,return size; } -static uint64_t strbuf_size (void *b) +static uint64_t reftable_buf_size (void *b) {return ((struct reftable_buf *)b)->len; } -static struct reftable_block_source_vtable strbuf_vtable = { - .size = &strbuf_size, - .read_block = &strbuf_read_block, - .return_block = &strbuf_return_block, - .close = &strbuf_close, +static struct reftable_block_source_vtable reftable_buf_vtable = { + .size = &reftable_buf_size, + .read_block = &reftable_buf_read_block, + .return_block = &reftable_buf_return_block, + .close = &reftable_buf_close, }; -void block_source_from_strbuf (struct reftable_block_source *bs, -struct reftable_buf *buf) +void block_source_from_buf (struct reftable_block_source *bs, +struct reftable_buf *buf) {assert (!bs->ops); - bs->ops = &strbuf_vtable; + bs->ops = &reftable_buf_vtable; bs->arg = buf; }
reftable/blocksource.h +2 -2
@@ -15,7 +15,7 @@ struct reftable_block_source;struct reftable_buf ;/* Create an in-memory block source for reading reftables */ -void block_source_from_strbuf (struct reftable_block_source *bs, -struct reftable_buf *buf); +void block_source_from_buf (struct reftable_block_source *bs, +struct reftable_buf *buf);#endif
t/unit-tests/t-reftable-block.c +4 -4
@@ -34,7 +34,7 @@ static void t_ref_block_read_write(void)REFTABLE_CALLOC_ARRAY (block.data, block_size);check (block.data !=NULL ); block.len = block_size; -block_source_from_strbuf (&block.source ,&buf); +block_source_from_buf (&block.source ,&buf); ret =block_writer_init (&bw,BLOCK_TYPE_REF , block.data, block_size, header_off,hash_size (GIT_SHA1_FORMAT_ID ));check (!ret);
@@ -128,7 +128,7 @@ static void t_log_block_read_write(void)REFTABLE_CALLOC_ARRAY (block.data, block_size);check (block.data !=NULL ); block.len = block_size; -block_source_from_strbuf (&block.source ,&buf); +block_source_from_buf (&block.source ,&buf); ret =block_writer_init (&bw,BLOCK_TYPE_LOG , block.data, block_size, header_off,hash_size (GIT_SHA1_FORMAT_ID ));check (!ret);
@@ -218,7 +218,7 @@ static void t_obj_block_read_write(void)REFTABLE_CALLOC_ARRAY (block.data, block_size);check (block.data !=NULL ); block.len = block_size; -block_source_from_strbuf (&block.source, &buf); +block_source_from_buf (&block.source, &buf); ret =block_writer_init (&bw,BLOCK_TYPE_OBJ , block.data, block_size, header_off,hash_size (GIT_SHA1_FORMAT_ID ));check (!ret);
@@ -302,7 +302,7 @@ static void t_index_block_read_write(void)REFTABLE_CALLOC_ARRAY (block.data, block_size);check (block.data !=NULL ); block.len = block_size; -block_source_from_strbuf (&block.source, &buf); +block_source_from_buf (&block.source, &buf); ret =block_writer_init (&bw,BLOCK_TYPE_INDEX , block.data, block_size, header_off,hash_size (GIT_SHA1_FORMAT_ID ));check (!ret);
t/unit-tests/t-reftable-merged.c +3 -3
@@ -35,7 +35,7 @@ merged_table_from_records(struct reftable_ref_record **refs,for (size_t i =0 ; i < n; i++) {t_reftable_write_to_buf (&buf[i], refs[i], sizes[i],NULL ,0 , &opts); -block_source_from_strbuf (&(*source)[i], &buf[i]); +block_source_from_buf (&(*source)[i], &buf[i]); err =reftable_reader_new (&(*readers)[i], &(*source)[i],"name" );
@@ -293,7 +293,7 @@ merged_table_from_log_records(struct reftable_log_record **logs,for (size_t i =0 ; i < n; i++) {t_reftable_write_to_buf (&buf[i],NULL ,0 , logs[i], sizes[i], &opts); -block_source_from_strbuf (&(*source)[i], &buf[i]); +block_source_from_buf (&(*source)[i], &buf[i]); err =reftable_reader_new (&(*readers)[i], &(*source)[i],"name" );
@@ -442,7 +442,7 @@ static void t_default_write_opts(void)check (!err);reftable_writer_free (w); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&rd, &source,"filename" );check (!err);
t/unit-tests/t-reftable-reader.c +2 -2
@@ -20,7 +20,7 @@ static int t_reader_seek_once(void)int ret;t_reftable_write_to_buf (&buf, records,ARRAY_SIZE (records),NULL ,0 ,NULL ); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); ret =reftable_reader_new (&reader, &source,"name" );check (!ret);
@@ -61,7 +61,7 @@ static int t_reader_reseek(void)int ret;t_reftable_write_to_buf (&buf, records,ARRAY_SIZE (records),NULL ,0 ,NULL ); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); ret =reftable_reader_new (&reader, &source,"name" );check (!ret);
t/unit-tests/t-reftable-readwrite.c +12 -12
@@ -24,7 +24,7 @@ static void t_buffer(void)int n;uint8_t in[] ="hello" ;reftable_buf_add (&buf, in,sizeof (in)); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); check_int(block_source_size(&source), ==, 6); n =block_source_read_block (&source, &out,0 ,sizeof (in)); check_int(n, ==, sizeof(in));
@@ -207,7 +207,7 @@ static void t_log_write_read(void)reftable_writer_free (w); w =NULL ; -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.log" );check (!err);
@@ -298,7 +298,7 @@ static void t_log_zlib_corruption(void)/* corrupt the data. */ buf.buf[50 ] ^=0x99 ; -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.log" );check (!err);
@@ -328,7 +328,7 @@ static void t_table_read_write_sequential(void)write_table (&names, &buf,N ,256 ,GIT_SHA1_FORMAT_ID ); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.ref" );check (!err);
@@ -380,7 +380,7 @@ static void t_table_read_api(void)write_table (&names, &buf,N ,256 ,GIT_SHA1_FORMAT_ID ); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.ref" );check (!err);
@@ -416,7 +416,7 @@ static void t_table_read_write_seek(int index, int hash_id)write_table (&names, &buf,N ,256 , hash_id); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.ref" );check (!err);
@@ -538,7 +538,7 @@ static void t_table_refs_for(int indexed)reftable_writer_free (w); w =NULL ; -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.ref" );check (!err);
@@ -600,7 +600,7 @@ static void t_write_empty_table(void) check_int(buf.len, ==, header_size(1) + footer_size(1)); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&rd, &source,"filename" );check (!err);
@@ -806,7 +806,7 @@ static void t_write_multiple_indices(void) check_int(stats->obj_stats.index_offset, >, 0); check_int(stats->log_stats.index_offset, >, 0); -block_source_from_strbuf (&source, &writer_buf); +block_source_from_buf (&source, &writer_buf); err =reftable_reader_new (&reader, &source,"filename" );check (!err);
@@ -863,7 +863,7 @@ static void t_write_multi_level_index(void) stats =reftable_writer_stats (writer); check_int(stats->ref_stats.max_index_level, ==, 2); -block_source_from_strbuf (&source, &writer_buf); +block_source_from_buf (&source, &writer_buf); err =reftable_reader_new (&reader, &source,"filename" );check (!err);
@@ -889,7 +889,7 @@ static void t_corrupt_table_empty(void)struct reftable_reader *reader;int err; -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.log" ); check_int(err, ==, REFTABLE_FORMAT_ERROR); }
@@ -903,7 +903,7 @@ static void t_corrupt_table(void)int err;reftable_buf_add (&buf, zeros,sizeof (zeros)); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.log" ); check_int(err, ==, REFTABLE_FORMAT_ERROR);
Signature · 1 line
2.47.0.dirty
PATCH 6/10 t/unit-tests: check for `reftable_buf` allocation errors
In reply to Patrick SteinhardtAdapt our unit tests to check for allocations errors.
Signed-off-by Patrick Steinhardt <[email protected]>
--- t/unit-tests/t-reftable-basics.c | 4 +- t/unit-tests/t-reftable-block.c | 4 +- t/unit-tests/t-reftable-readwrite.c | 8 ++-- t/unit-tests/t-reftable-record.c | 14 +++---- t/unit-tests/t-reftable-stack.c | 58 ++++++++++++++--------------- 5 files changed, 44 insertions(+), 44 deletions(-)
t/unit-tests/t-reftable-basics.c +2 -2
@@ -113,8 +113,8 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED) };for (size_t i =0 ; i <ARRAY_SIZE (cases); i++) { -reftable_buf_addstr (&a, cases[i].a); -reftable_buf_addstr (&b, cases[i].b); +check (!reftable_buf_addstr (&a, cases[i].a)); +check (!reftable_buf_addstr (&b, cases[i].b)); check_int(common_prefix_size(&a, &b), ==, cases[i].want);reftable_buf_reset (&a);reftable_buf_reset (&b);
t/unit-tests/t-reftable-block.c +2 -2
@@ -167,7 +167,7 @@ static void t_log_block_read_write(void)for (i =0 ; i <N ; i++) {block_iter_reset (&it);reftable_buf_reset (&want); -reftable_buf_addstr (&want, recs[i].u.log.refname); +check (!reftable_buf_addstr (&want, recs[i].u.log.refname)); ret =block_iter_seek_key (&it, &br, &want); check_int(ret, ==, 0);
@@ -314,7 +314,7 @@ static void t_index_block_read_write(void)reftable_buf_init (&recs[i].u.idx.last_key); recs[i].type =BLOCK_TYPE_INDEX ; -reftable_buf_addstr (&recs[i].u.idx.last_key, buf); +check (!reftable_buf_addstr (&recs[i].u.idx.last_key, buf)); recs[i].u.idx.offset = i; ret =block_writer_add (&bw, &recs[i]);
t/unit-tests/t-reftable-readwrite.c +4 -4
@@ -23,7 +23,7 @@ static void t_buffer(void)struct reftable_block out = {0 };int n;uint8_t in[] ="hello" ; -reftable_buf_add (&buf, in,sizeof (in)); +check (!reftable_buf_add (&buf, in,sizeof (in)));block_source_from_buf (&source, &buf); check_int(block_source_size(&source), ==, 6); n =block_source_read_block (&source, &out,0 ,sizeof (in));
@@ -443,8 +443,8 @@ static void t_table_read_write_seek(int index, int hash_id)reftable_iterator_destroy (&it); } -reftable_buf_addstr (&pastLast, names[N -1 ]); -reftable_buf_addstr (&pastLast,"/" ); +check (!reftable_buf_addstr (&pastLast, names[N -1 ])); +check (!reftable_buf_addstr (&pastLast,"/" )); err =reftable_reader_init_ref_iterator (reader, &it);check (!err);
@@ -901,7 +901,7 @@ static void t_corrupt_table(void)struct reftable_block_source source = {0 };struct reftable_reader *reader;int err; -reftable_buf_add (&buf, zeros,sizeof (zeros)); +check (!reftable_buf_add (&buf, zeros,sizeof (zeros)));block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.log" );
t/unit-tests/t-reftable-record.c +7 -7
@@ -335,14 +335,14 @@ static void t_key_roundtrip(void)int n, m;uint8_t rt_extra; -reftable_buf_addstr (&last_key,"refs/heads/master" ); -reftable_buf_addstr (&key,"refs/tags/bla" ); +check (!reftable_buf_addstr (&last_key,"refs/heads/master" )); +check (!reftable_buf_addstr (&key,"refs/tags/bla" )); extra =6 ; n =reftable_encode_key (&restart, dest, last_key, key, extra);check (!restart); check_int(n, >, 0); -reftable_buf_addstr (&roundtrip,"refs/heads/master" ); +check (!reftable_buf_addstr (&roundtrip,"refs/heads/master" )); m =reftable_decode_key (&roundtrip, &rt_extra, dest); check_int(n, ==, m);check (!reftable_buf_cmp (&key, &roundtrip));
@@ -469,9 +469,9 @@ static void t_reftable_index_record_comparison(void) .u.idx.last_key = REFTABLE_BUF_INIT, }, }; -reftable_buf_addstr (&in[0 ].u.idx.last_key,"refs/heads/master" ); -reftable_buf_addstr (&in[1 ].u.idx.last_key,"refs/heads/master" ); -reftable_buf_addstr (&in[2 ].u.idx.last_key,"refs/heads/branch" ); +check (!reftable_buf_addstr (&in[0 ].u.idx.last_key,"refs/heads/master" )); +check (!reftable_buf_addstr (&in[1 ].u.idx.last_key,"refs/heads/master" )); +check (!reftable_buf_addstr (&in[2 ].u.idx.last_key,"refs/heads/branch" ));check (!reftable_record_equal (&in[0 ], &in[1 ],GIT_SHA1_RAWSZ ));check (!reftable_record_cmp (&in[0 ], &in[1 ]));
@@ -510,7 +510,7 @@ static void t_reftable_index_record_roundtrip(void)int n, m;uint8_t extra; -reftable_buf_addstr (&in.u.idx.last_key,"refs/heads/master" ); +check (!reftable_buf_addstr (&in.u.idx.last_key,"refs/heads/master" ));reftable_record_key (&in, &key);t_copy (&in);
t/unit-tests/t-reftable-stack.c +29 -29
106 lines not on this page · raw message
Signature · 1 line
2.47.0.dirty
PATCH 7/10 reftable/stack: adapt `format_name()` to handle allocation failures
In reply to Patrick SteinhardtThe format_name() function cannot pass any errors to the caller as it has a void return type. Adapt it and its callers such that we can handle errors and start handling allocation failures.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/stack.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-)
reftable/stack.c +19 -9
@@ -623,14 +623,14 @@ int reftable_stack_add(struct reftable_stack *st,return 0 ; } -static void format_name (struct reftable_buf *dest,uint64_t min,uint64_t max) +static int format_name (struct reftable_buf *dest,uint64_t min,uint64_t max) {char buf[100 ];uint32_t rnd = (uint32_t )git_rand ();snprintf (buf,sizeof (buf),"0x%012" PRIx64"-0x%012" PRIx64"-%08x" , min, max, rnd);reftable_buf_reset (dest); -reftable_buf_addstr (dest, buf); +return reftable_buf_addstr (dest, buf); }struct reftable_addition {
@@ -846,7 +846,10 @@ int reftable_addition_add(struct reftable_addition *add,int tab_fd;reftable_buf_reset (&next_name); -format_name (&next_name, add->next_update_index, add->next_update_index); + + err =format_name (&next_name, add->next_update_index, add->next_update_index); +if (err <0 ) + goto done;stack_filename (&temp_tab_file_name, add->stack, next_name.buf);reftable_buf_addstr (&temp_tab_file_name,".temp.XXXXXX" );
@@ -893,7 +896,9 @@ int reftable_addition_add(struct reftable_addition *add, goto done; } -format_name (&next_name, wr->min_update_index, wr->max_update_index); + err =format_name (&next_name, wr->min_update_index, wr->max_update_index); +if (err <0 ) + goto done;reftable_buf_addstr (&next_name,".ref" );stack_filename (&tab_file_name, add->stack, next_name.buf);
@@ -944,9 +949,11 @@ static int stack_compact_locked(struct reftable_stack *st,struct tempfile *tab_file;int tab_fd, err =0 ; -format_name (&next_name, -reftable_reader_min_update_index (st->readers[first]), -reftable_reader_max_update_index (st->readers[last])); + err =format_name (&next_name,reftable_reader_min_update_index (st->readers[first]), +reftable_reader_max_update_index (st->readers[last])); +if (err <0 ) + goto done; +stack_filename (&tab_file_path, st, next_name.buf);reftable_buf_addstr (&tab_file_path,".temp.XXXXXX" );
@@ -1370,8 +1377,11 @@ static int stack_compact_range(struct reftable_stack *st,* it into place now. */ if (!is_empty_table) { -format_name (&new_table_name, st->readers[first]->min_update_index, - st->readers[last]->max_update_index); + err =format_name (&new_table_name, st->readers[first]->min_update_index, + st->readers[last]->max_update_index); +if (err <0 ) + goto done; +reftable_buf_addstr (&new_table_name,".ref" );stack_filename (&new_table_path, st, new_table_name.buf);
Signature · 1 line
2.47.0.dirty
PATCH 8/10 reftable/record: adapt `reftable_record_key()` to handle allocation failures
In reply to Patrick SteinhardtThe reftable_record_key() function cannot pass any errors to the caller as it has a void return type. Adapt it and its callers such that we can handle errors and start handling allocation failures.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/block.c | 20 +++++++++++++++----- reftable/reader.c | 8 ++++++-- reftable/record.c | 32 ++++++++++++++++++++------------ reftable/record.h | 4 ++-- reftable/writer.c | 5 ++++- 5 files changed, 47 insertions(+), 22 deletions(-)
reftable/block.c +15 -5
@@ -111,9 +111,12 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)int is_restart =0 ;struct reftable_buf key =REFTABLE_BUF_INIT ;int n =0 ; -int err =-1 ; +int err; + + err =reftable_record_key (rec, &key); +if (err <0 ) + goto done; -reftable_record_key (rec, &key);if (!key.len) { err =REFTABLE_API_ERROR ; goto done;
@@ -121,13 +124,17 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec) n =reftable_encode_key (&is_restart, out, last, key,reftable_record_val_type (rec)); -if (n <0 ) +if (n <0 ) { + err =-1 ; goto done; + }string_view_consume (&out, n); n =reftable_record_encode (rec, out, w->hash_size); -if (n <0 ) +if (n <0 ) { + err =-1 ; goto done; + }string_view_consume (&out, n); err = block_writer_register_restart(w, start.len - out.len, is_restart,
@@ -522,6 +529,10 @@ int block_iter_seek_key(struct block_iter *it, const struct block_reader *br, goto done; } + err =reftable_record_key (&rec, &it->last_key); +if (err <0 ) + goto done; +/* * Check whether the current key is greater or equal to the * sought-after key. In case it is greater we know that the
@@ -536,7 +547,6 @@ int block_iter_seek_key(struct block_iter *it, const struct block_reader *br,* to `last_key` now, and naturally all keys share a prefix * with themselves. */ -reftable_record_key (&rec, &it->last_key);if (reftable_buf_cmp (&it->last_key, want) >=0 ) { it->next_off = prev_off; goto done;
reftable/reader.c +6 -2
@@ -356,7 +356,9 @@ static int table_iter_seek_linear(struct table_iter *ti,int err;reftable_record_init (&rec,reftable_record_type (want)); -reftable_record_key (want, &want_key); + err =reftable_record_key (want, &want_key); +if (err <0 ) + goto done;/* * First we need to locate the block that must contain our record. To
@@ -439,7 +441,9 @@ static int table_iter_seek_indexed(struct table_iter *ti, };int err; -reftable_record_key (rec, &want_index.u.idx.last_key); + err =reftable_record_key (rec, &want_index.u.idx.last_key); +if (err <0 ) + goto done;/* * The index may consist of multiple levels, where each level may have
reftable/record.c +20 -12
@@ -207,12 +207,12 @@ int reftable_decode_key(struct reftable_buf *last_key, uint8_t *extra,return start_len - in.len; } -static void reftable_ref_record_key (const void *r,struct reftable_buf *dest) +static int reftable_ref_record_key (const void *r,struct reftable_buf *dest) {const struct reftable_ref_record *rec = (const struct reftable_ref_record *)r;reftable_buf_reset (dest); -reftable_buf_addstr (dest, rec->refname); +return reftable_buf_addstr (dest, rec->refname); } static int reftable_ref_record_copy_from(void *rec, const void *src_rec,
@@ -465,12 +465,12 @@ static struct reftable_record_vtable reftable_ref_record_vtable = { .cmp = &reftable_ref_record_cmp_void, }; -static void reftable_obj_record_key (const void *r,struct reftable_buf *dest) +static int reftable_obj_record_key (const void *r,struct reftable_buf *dest) {const struct reftable_obj_record *rec = (const struct reftable_obj_record *)r;reftable_buf_reset (dest); -reftable_buf_add (dest, rec->hash_prefix, rec->hash_prefix_len); +return reftable_buf_add (dest, rec->hash_prefix, rec->hash_prefix_len); }static void reftable_obj_record_release (void *rec)
@@ -664,19 +664,27 @@ static struct reftable_record_vtable reftable_obj_record_vtable = { .cmp = &reftable_obj_record_cmp_void, }; -static void reftable_log_record_key (const void *r,struct reftable_buf *dest) +static int reftable_log_record_key (const void *r,struct reftable_buf *dest) {const struct reftable_log_record *rec = (const struct reftable_log_record *)r; -int len =strlen (rec->refname); +int len =strlen (rec->refname), err;uint8_t i64[8 ];uint64_t ts =0 ; +reftable_buf_reset (dest); -reftable_buf_add (dest, (uint8_t *)rec->refname, len +1 ); + err =reftable_buf_add (dest, (uint8_t *)rec->refname, len +1 ); +if (err <0 ) +return err; ts = (~ts) - rec->update_index;put_be64 (&i64[0 ], ts); -reftable_buf_add (dest, i64,sizeof (i64)); + + err =reftable_buf_add (dest, i64,sizeof (i64)); +if (err <0 ) +return err; + +return 0 ; } static int reftable_log_record_copy_from(void *rec, const void *src_rec,
@@ -1027,11 +1035,11 @@ static struct reftable_record_vtable reftable_log_record_vtable = { .cmp = &reftable_log_record_cmp_void, }; -static void reftable_index_record_key (const void *r,struct reftable_buf *dest) +static int reftable_index_record_key (const void *r,struct reftable_buf *dest) {const struct reftable_index_record *rec = r;reftable_buf_reset (dest); -reftable_buf_add (dest, rec->last_key.buf, rec->last_key.len); +return reftable_buf_add (dest, rec->last_key.buf, rec->last_key.len); } static int reftable_index_record_copy_from(void *rec, const void *src_rec,
@@ -1124,9 +1132,9 @@ static struct reftable_record_vtable reftable_index_record_vtable = { .cmp = &reftable_index_record_cmp, }; -void reftable_record_key (struct reftable_record *rec,struct reftable_buf *dest) +int reftable_record_key (struct reftable_record *rec,struct reftable_buf *dest) { -reftable_record_vtable (rec)->key (reftable_record_data (rec), dest); +return reftable_record_vtable (rec)->key (reftable_record_data (rec), dest); } int reftable_record_encode(struct reftable_record *rec, struct string_view dest,
reftable/record.h +2 -2
@@ -40,7 +40,7 @@ int put_var_int(struct string_view *dest, uint64_t val);/* Methods for records. */ struct reftable_record_vtable {/* encode the key of to a uint8_t reftable_buf. */ -void (*key)(const void *rec,struct reftable_buf *dest); +int (*key)(const void *rec,struct reftable_buf *dest);/* The record type of ('r' for ref). */ uint8_t type;
@@ -137,7 +137,7 @@ void reftable_record_init(struct reftable_record *rec, uint8_t typ);/* see struct record_vtable */ int reftable_record_cmp (struct reftable_record *a,struct reftable_record *b);int reftable_record_equal (struct reftable_record *a,struct reftable_record *b,int hash_size); -void reftable_record_key (struct reftable_record *rec,struct reftable_buf *dest); +int reftable_record_key (struct reftable_record *rec,struct reftable_buf *dest);int reftable_record_copy_from (struct reftable_record *rec,struct reftable_record *src,int hash_size);uint8_t reftable_record_val_type (struct reftable_record *rec);
reftable/writer.c +4 -1
@@ -249,7 +249,10 @@ static int writer_add_record(struct reftable_writer *w,struct reftable_buf key =REFTABLE_BUF_INIT ;int err; -reftable_record_key (rec, &key); + err =reftable_record_key (rec, &key); +if (err <0 ) + goto done; +if (reftable_buf_cmp (&w->last_key, &key) >=0 ) { err =REFTABLE_API_ERROR ; goto done;
Signature · 1 line
2.47.0.dirty
PATCH 9/10 reftable/stack: adapt `stack_filename()` to handle allocation failures
In reply to Patrick SteinhardtThe stack_filename() function cannot pass any errors to the caller as it has a void return type. Adapt it and its callers such that we can handle errors and start handling allocation failures.
There are two interesting edge cases in reftable_stack_destroy() and reftable_addition_close(). Both of these are trying to tear down their respective structures, and while doing so they try to unlink some of the tables they have been keeping alive. Any earlier attempts to do that may fail on Windows because it keeps us from deleting such tables while they are still open, and thus we re-try on close. It's okay and even expected that this can fail when the tables are still open by another process, so we handle the allocation failures gracefully and just skip over any file whose name we couldn't figure out.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/stack.c | 62 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 17 deletions(-)
reftable/stack.c +45 -17
@@ -31,13 +31,16 @@ static void reftable_addition_close(struct reftable_addition *add);static int reftable_stack_reload_maybe_reuse (struct reftable_stack *st,int reuse_open); -static void stack_filename (struct reftable_buf *dest,struct reftable_stack *st, -const char *name) +static int stack_filename (struct reftable_buf *dest,struct reftable_stack *st, +const char *name) { +int err;reftable_buf_reset (dest); -reftable_buf_addstr (dest, st->reftable_dir); -reftable_buf_addstr (dest,"/" ); -reftable_buf_addstr (dest, name); +if ((err =reftable_buf_addstr (dest, st->reftable_dir)) <0 || + (err =reftable_buf_addstr (dest,"/" )) <0 || + (err =reftable_buf_addstr (dest, name)) <0 ) +return err; +return 0 ; }static ssize_t reftable_fd_write (void *arg,const void *data,size_t sz)
@@ -211,13 +214,16 @@ void reftable_stack_destroy(struct reftable_stack *st)struct reftable_buf filename =REFTABLE_BUF_INIT ;for (i =0 ; i < st->readers_len; i++) {const char *name =reader_name (st->readers[i]); +int try_unlinking =1 ; +reftable_buf_reset (&filename);if (names && !has_name (names, name)) { -stack_filename (&filename, st, name); +if (stack_filename (&filename, st, name) <0 ) + try_unlinking =0 ; }reftable_reader_decref (st->readers[i]); -if (filename.len) { +if (try_unlinking && filename.len) {/* On Windows, can only unlink after closing. */ unlink (filename.buf); }
@@ -310,7 +316,10 @@ static int reftable_stack_reload_once(struct reftable_stack *st,if (!rd) {struct reftable_block_source src = {NULL }; -stack_filename (&table_path, st, name); + + err =stack_filename (&table_path, st, name); +if (err <0 ) + goto done; err =reftable_block_source_from_file (&src, table_path.buf);
@@ -341,7 +350,11 @@ static int reftable_stack_reload_once(struct reftable_stack *st,for (i =0 ; i < cur_len; i++) {if (cur[i]) {const char *name =reader_name (cur[i]); -stack_filename (&table_path, st, name); + + err =stack_filename (&table_path, st, name); +if (err <0 ) + goto done; +reftable_reader_decref (cur[i]);unlink (table_path.buf); }
@@ -700,8 +713,8 @@ static void reftable_addition_close(struct reftable_addition *add)size_t i;for (i =0 ; i < add->new_tables_len; i++) { -stack_filename (&nm, add->stack, add->new_tables[i]); -unlink (nm.buf); +if (!stack_filename (&nm, add->stack, add->new_tables[i])) +unlink (nm.buf);reftable_free (add->new_tables[i]); add->new_tables[i] =NULL ; }
@@ -851,7 +864,9 @@ int reftable_addition_add(struct reftable_addition *add,if (err <0 ) goto done; -stack_filename (&temp_tab_file_name, add->stack, next_name.buf); + err =stack_filename (&temp_tab_file_name, add->stack, next_name.buf); +if (err <0 ) + goto done;reftable_buf_addstr (&temp_tab_file_name,".temp.XXXXXX" ); tab_file =mks_tempfile (temp_tab_file_name.buf);
@@ -900,7 +915,10 @@ int reftable_addition_add(struct reftable_addition *add,if (err <0 ) goto done;reftable_buf_addstr (&next_name,".ref" ); -stack_filename (&tab_file_name, add->stack, next_name.buf); + + err =stack_filename (&tab_file_name, add->stack, next_name.buf); +if (err <0 ) + goto done;/* On windows, this relies on rand() picking a unique destination name.
@@ -954,7 +972,9 @@ static int stack_compact_locked(struct reftable_stack *st,if (err <0 ) goto done; -stack_filename (&tab_file_path, st, next_name.buf); + err =stack_filename (&tab_file_path, st, next_name.buf); +if (err <0 ) + goto done;reftable_buf_addstr (&tab_file_path,".temp.XXXXXX" ); tab_file =mks_tempfile (tab_file_path.buf);
@@ -1174,7 +1194,9 @@ static int stack_compact_range(struct reftable_stack *st, }for (i = last +1 ; i > first; i--) { -stack_filename (&table_name, st,reader_name (st->readers[i -1 ])); + err =stack_filename (&table_name, st,reader_name (st->readers[i -1 ])); +if (err <0 ) + goto done; err =hold_lock_file_for_update (&table_locks[nlocks], table_name.buf,LOCK_NO_DEREF );
@@ -1383,7 +1405,10 @@ static int stack_compact_range(struct reftable_stack *st, goto done;reftable_buf_addstr (&new_table_name,".ref" ); -stack_filename (&new_table_path, st, new_table_name.buf); + + err =stack_filename (&new_table_path, st, new_table_name.buf); +if (err <0 ) + goto done; err =rename_tempfile (&new_table, new_table_path.buf);if (err <0 ) {
@@ -1677,7 +1702,10 @@ static void remove_maybe_stale_table(struct reftable_stack *st, uint64_t max,struct reftable_block_source src = {NULL };struct reftable_reader *rd =NULL ;struct reftable_buf table_path =REFTABLE_BUF_INIT ; -stack_filename (&table_path, st, name); + + err =stack_filename (&table_path, st, name); +if (err <0 ) + goto done; err =reftable_block_source_from_file (&src, table_path.buf);if (err <0 )
Signature · 1 line
2.47.0.dirty
PATCH 10/10 reftable: handle trivial `reftable_buf` errors
In reply to Patrick SteinhardtConvert the reftable library such that we handle failures with the new reftable_buf interfaces.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/block.c | 9 +++++++-- reftable/iter.c | 5 ++++- reftable/reader.c | 5 ++++- reftable/record.c | 32 +++++++++++++++++++++++-------- reftable/stack.c | 49 +++++++++++++++++++++++++++++++---------------- reftable/writer.c | 48 +++++++++++++++++++++++++++++++++------------- 6 files changed, 107 insertions(+), 41 deletions(-)
reftable/block.c +7 -2
@@ -40,7 +40,9 @@ int footer_size(int version)static int block_writer_register_restart (struct block_writer *w,int n,int is_restart,struct reftable_buf *key) { -int rlen = w->restart_len; +int rlen, err; + + rlen = w->restart_len;if (rlen >=MAX_RESTARTS ) { is_restart =0 ; }
@@ -60,7 +62,10 @@ static int block_writer_register_restart(struct block_writer *w, int n, w->next += n;reftable_buf_reset (&w->last_key); -reftable_buf_add (&w->last_key, key->buf, key->len); + err =reftable_buf_add (&w->last_key, key->buf, key->len); +if (err <0 ) +return err; + w->entries++;return 0 ; }
reftable/iter.c +4 -1
@@ -197,7 +197,10 @@ int indexed_table_ref_iter_new(struct indexed_table_ref_iter **dest, *itr = empty; itr->r = r; -reftable_buf_add (&itr->oid, oid, oid_len); + + err =reftable_buf_add (&itr->oid, oid, oid_len); +if (err <0 ) + goto out; itr->offsets = offsets; itr->offset_len = offset_len;
reftable/reader.c +4 -1
@@ -769,7 +769,10 @@ static int reftable_reader_refs_for_unindexed(struct reftable_reader *r, } *filter = empty; -reftable_buf_add (&filter->oid, oid, oid_len); + err =reftable_buf_add (&filter->oid, oid, oid_len); +if (err <0 ) + goto out; +iterator_from_table_iter (&filter->it, ti);iterator_from_filtering_ref_iterator (it, filter);
reftable/record.c +24 -8
@@ -102,7 +102,9 @@ static int decode_string(struct reftable_buf *dest, struct string_view in) {int start_len = in.len;uint64_t tsize =0 ; -int n =get_var_int (&tsize, &in); +int n, err; + + n =get_var_int (&tsize, &in);if (n <=0 )return -1 ;string_view_consume (&in, n);
@@ -110,7 +112,10 @@ static int decode_string(struct reftable_buf *dest, struct string_view in)return -1 ;reftable_buf_reset (dest); -reftable_buf_add (dest, in.buf, tsize); + err =reftable_buf_add (dest, in.buf, tsize); +if (err <0 ) +return err; +string_view_consume (&in, tsize);return start_len - in.len;
@@ -189,7 +194,7 @@ int reftable_decode_key(struct reftable_buf *last_key, uint8_t *extra,int start_len = in.len;uint64_t prefix_len =0 ;uint64_t suffix_len =0 ; -int n; +int err, n; n =reftable_decode_keylen (in, &prefix_len, &suffix_len, extra);if (n <0 )
@@ -200,8 +205,14 @@ int reftable_decode_key(struct reftable_buf *last_key, uint8_t *extra, prefix_len > last_key->len)return -1 ; -reftable_buf_setlen (last_key, prefix_len); -reftable_buf_add (last_key, in.buf, suffix_len); + err =reftable_buf_setlen (last_key, prefix_len); +if (err <0 ) +return err; + + err =reftable_buf_add (last_key, in.buf, suffix_len); +if (err <0 ) +return err; +string_view_consume (&in, suffix_len);return start_len - in.len;
@@ -1047,9 +1058,12 @@ static int reftable_index_record_copy_from(void *rec, const void *src_rec, {struct reftable_index_record *dst = rec;const struct reftable_index_record *src = src_rec; +int err;reftable_buf_reset (&dst->last_key); -reftable_buf_add (&dst->last_key, src->last_key.buf, src->last_key.len); + err =reftable_buf_add (&dst->last_key, src->last_key.buf, src->last_key.len); +if (err <0 ) +return err; dst->offset = src->offset;return 0 ;
@@ -1090,10 +1104,12 @@ static int reftable_index_record_decode(void *rec, struct reftable_buf key, {struct string_view start = in;struct reftable_index_record *r = rec; -int n =0 ; +int err, n =0 ;reftable_buf_reset (&r->last_key); -reftable_buf_add (&r->last_key, key.buf, key.len); + err =reftable_buf_add (&r->last_key, key.buf, key.len); +if (err <0 ) +return err; n =get_var_int (&r->offset, &in);if (n <0 )
reftable/stack.c +33 -16
97 lines not on this page · raw message
reftable/writer.c +35 -13
96 lines not on this page · raw message
Signature · 1 line
2.47.0.dirty
Re: [PATCH 02/10] reftable: stop using `strbuf_addf()`
In reply to Patrick SteinhardtPatrick Steinhardt <[email protected]> writes:
[snip]
Quoted 20 lines
t/unit-tests/t-reftable-readwrite.c +2
-1@@ -753,12 +753,13 @@ static void t_write_multiple_indices(void)struct reftable_write_options opts = { .block_size =100 , }; -struct strbuf writer_buf =STRBUF_INIT , buf =STRBUF_INIT ; +struct strbuf writer_buf =STRBUF_INIT ;struct reftable_block_source source = {0 };struct reftable_iterator it = {0 };const struct reftable_stats *stats;struct reftable_writer *writer;struct reftable_reader *reader; +char buf[128 ];int err, i; writer =t_reftable_strbuf_writer (&writer_buf, &opts);
t/unit-tests/t-reftable-readwrite.c static void t_write_multiple_indices(void) · line 773
Quoted 4 lines
t/unit-tests/t-reftable-readwrite.c +0
-1@@ -770,9 +771,8 @@ static void t_write_multiple_indices(void) .value.val1 = {i}, }; - strbuf_reset(&buf);
Here, it is okay to remove this, since we define our own buf array.
Quoted 8 lines
t/unit-tests/t-reftable-readwrite.c +2
-2-strbuf_addf (&buf,"refs/heads/%04d" , i); - ref.refname = buf.buf, +snprintf (buf,sizeof (buf),"refs/heads/%04d" , i); + ref.refname = buf; err =reftable_writer_add_ref (writer, &ref);check (!err);@@ -788,9 +788,8 @@ static void t_write_multiple_indices(void)
[snip]
t/unit-tests/t-reftable-readwrite.c static void t_reftable_stack_auto_compaction_with_locked_tables(void) · line 1080
Quoted 5 lines
t/unit-tests/t-reftable-readwrite.c +0
-1@@ -1077,8 +1078,10 @@ static void t_reftable_stack_auto_compaction_with_locked_tables(void)* size, we expect that auto-compaction will want to compact all of the * tables. Locking any of the tables will keep it from doing so. */ -strbuf_reset (&buf);
However here it is different, since we still use the strbuf. I guess it should be okay, since 'buf' is initialized using 'STRBUF_INIT' and that still keeps the buf.len to 0.
t/unit-tests/t-reftable-readwrite.c static void t_reftable_stack_auto_compaction_with_locked_tables(void) · line 1086
Quoted 7 lines
t/unit-tests/t-reftable-readwrite.c +4
-1-strbuf_addf (&buf,"%s/%s.lock" , dir, st->readers[2 ]->name); +strbuf_addstr (&buf, dir); +strbuf_addstr (&buf,"/" ); +strbuf_addstr (&buf, st->readers[2 ]->name); +strbuf_addstr (&buf,".lock" );write_file_buf (buf.buf,"" ,0 );
So when we do 'strbuf_addstr(&buf, ...)' it should allocate the required memory. But the reset removal did catch my eye.
[snip]
Re: [PATCH 03/10] reftable/basics: provide new `reftable_buf` interface
In reply to Patrick SteinhardtQuoted 6 lines · Patrick Steinhardt
Implement a new
reftable_bufinterface that will replace Git's ownstrbufinterface. This is done due to three reasons:- The
strbufinterfaces do not handle memory allocation failures and instead causes us to die. This is okay in the context of Git, but is not context of the reftable library, which is supposed to be usable
s/not/not in the/
Quoted 15 lines
by third-party applications.
- The
strbufinterface is quite deeply tied into Git, which makes it hard to use the reftable library as a standalone library. Any dependent would have to carefully extract the relevant parts of it to make things work, which is not all that sensible.- The
strbufinterface does not use the pluggable allocators that can be set up viarefatble_set_alloc().So we have good reasons to use our own type, and the implementation is rather trivial. Implement our own type. Conversion of the reftable library will be handled in subsequent commits.
Signed-off-by Patrick Steinhardt <[email protected]>
[snip]
Quoted 14 lines
reftable/basics.h +6
-0@@ -16,6 +16,22 @@ license that can be found in the LICENSE file or at#include "system.h" #include "reftable-basics.h" +struct reftable_buf { +size_t alloc; +size_t len; +char *buf; +}; +#define REFTABLE_BUF_INIT { 0 }
reftable/basics.h license that can be found in the LICENSE file or at · line 34
Quoted 10 lines
reftable/basics.h +10
-0+ +void reftable_buf_init (struct reftable_buf *buf); +void reftable_buf_release (struct reftable_buf *buf); +void reftable_buf_reset (struct reftable_buf *buf); +int reftable_buf_setlen (struct reftable_buf *buf,size_t len); +int reftable_buf_cmp (const struct reftable_buf *a,const struct reftable_buf *b); +int reftable_buf_add (struct reftable_buf *buf,const void *data,size_t len); +int reftable_buf_addstr (struct reftable_buf *buf,const char *s); +char *reftable_buf_detach (struct reftable_buf *buf); +
Nit: would be nice to have some comments explaining the functions here. I know most of them are self-explanatory and similar to strbuf, but since this is supposed to be isolated, it would be nice.
[snip]
Re: [PATCH 04/10] reftable: convert from `strbuf` to `reftable_buf`
In reply to Patrick SteinhardtQuoted 5 lines · Patrick Steinhardt
Convert the reftable library to use the
reftable_bufinterface instead of thestrbufinterface. This is a mechanical change via sed(1) and does not yet handle allocation failures. These will be addressed in subsequent commits.
Nit: Would be nice to list the sed command used here, so reviewers can review that instead.
[snip]
reftable/basics.h char *reftable_strdup(const char *str); · line 107
Quoted 9 lines
reftable/basics.h +0
-1@@ -104,8 +104,7 @@ char *reftable_strdup(const char *str);#endif /* Find the longest shared prefix size of `a` and `b` */ -struct strbuf ;
I guess this is the only manual part of this commit, would be nice to mention this in the message.
Quoted 5 lines
reftable/basics.h +1
-1-int common_prefix_size (struct strbuf *a,struct strbuf *b); +int common_prefix_size (struct reftable_buf *a,struct reftable_buf *b);int hash_size (uint32_t id);
[snip]
Quoted 10 lines · Patrick Steinhardt
Hi,
this is the second patch series on my quest to make the reftable library become a standalone library again that can be used by libgit2 without pulling in all kinds of dependencies from the Git codebase. This part makes us lose the dependency on
struct strbuf, which is done due to three reasons:- To make us independent of libgit.a.
Quoted 8 lines
- To ensure that we use the pluggable allocators that users can set up
via `reftable_set_alloc()`.- To make it possible to handle memory allocation failures.
While this leads to some duplication, we're only talking about ~70 lines of code.
I only have a few small comments, but overall this series looks good. Thanks
[snip]
v2 cover reftable: stop using `struct strbuf`
In reply to Patrick SteinhardtHi,
this is the second part of my patch series that stop using struct strbuf in the reftable library. This is done such that the reftable library becomes standalone again and so that we can use the pluggable allocators part of the library.
Changes compared to v1:
- Point out a while-at-it refactoring when getting rid of
`strbuf_addf()`.
- Fix grammar issue in another commit message.
- Use REFTABLE_OUT_OF_MEMORY_ERROR error codes on allocation failure.
- Add documentation for struct reftable_buf functions.
- Clarify that the conversion to use the new interface is not wholly mechanical via sed(1).
Thanks!
Patrick
Patrick Steinhardt (10):
reftable: stop using strbuf_addbuf()
reftable: stop using strbuf_addf()
reftable/basics: provide new reftable_buf interface
reftable: convert from strbuf to reftable_buf reftable/blocksource: adapt interface name
t/unit-tests: check for reftable_buf allocation errors reftable/stack: adapt format_name() to handle allocation failures reftable/record: adapt reftable_record_key() to handle allocation
failures
reftable/stack: adapt stack_filename() to handle allocation failures
reftable: handle trivial reftable_buf errors
reftable/basics.c | 76 +++++++++- reftable/basics.h | 59 +++++++- reftable/block.c | 61 +++++--- reftable/block.h | 14 +- reftable/blocksource.c | 30 ++-- reftable/blocksource.h | 5 +- reftable/iter.c | 9 +- reftable/iter.h | 8 +- reftable/reader.c | 27 ++-- reftable/record.c | 114 ++++++++------ reftable/record.h | 21 +-- reftable/stack.c | 221 ++++++++++++++++++---------- reftable/system.h | 1 - reftable/writer.c | 102 ++++++++----- reftable/writer.h | 2 +- t/unit-tests/lib-reftable.c | 4 +- t/unit-tests/lib-reftable.h | 7 +- t/unit-tests/t-reftable-basics.c | 16 +- t/unit-tests/t-reftable-block.c | 53 +++---- t/unit-tests/t-reftable-merged.c | 32 ++-- t/unit-tests/t-reftable-reader.c | 12 +- t/unit-tests/t-reftable-readwrite.c | 134 +++++++++-------- t/unit-tests/t-reftable-record.c | 74 +++++----- t/unit-tests/t-reftable-stack.c | 96 ++++++------ 24 files changed, 726 insertions(+), 452 deletions(-)
Range-diff against v1:
1: 7408482c152 = 1: 7408482c152 reftable: stop using `strbuf_addbuf()` 2: abc28d7664f ! 2: 6a7333b275e reftable: stop using `strbuf_addf()`
@@ Commit message
strbuf. Get rid of the seldomly-used strbuf_addf() function such that we have to reimplement one less function.
+ While at it, remove a useless call to strbuf_reset() in
+ t_reftable_stack_auto_compaction_with_locked_tables(). We don't write
+ to the buffer before this and initialize it with STRBUF_INIT, so there
+ is no need to reset anything.
+
Signed-off-by Patrick Steinhardt <[email protected]>
## reftable/stack.c ##
3: 24e31619b93 ! 3: 0ddc8c0c896 reftable/basics: provide new reftable_buf interface
@@ Commit message
- The `strbuf` interfaces do not handle memory allocation failures and
instead causes us to die. This is okay in the context of Git, but is
- not context of the reftable library, which is supposed to be usable
- by third-party applications.
+ not in the context of the reftable library, which is supposed to be
+ usable by third-party applications.
- The `strbuf` interface is quite deeply tied into Git, which makes it
hard to use the reftable library as a standalone library. Any
@@ Commit message
Signed-off-by: Patrick Steinhardt <[email protected]>
## reftable/basics.c ##
+@@ reftable/basics.c: license that can be found in the LICENSE file or at
+ #define REFTABLE_ALLOW_BANNED_ALLOCATORS
+ #include "basics.h"
+ #include "reftable-basics.h"
++#include "reftable-error.h"
+
+ static void *(*reftable_malloc_ptr)(size_t sz);
+ static void *(*reftable_realloc_ptr)(void *, size_t);
@@ reftable/basics.c: void reftable_set_alloc(void *(*malloc)(size_t),
reftable_free_ptr = free;
}
@@ reftable/basics.c: void reftable_set_alloc(void *(*malloc)(size_t),
+ char *reallocated = buf->buf;
+ REFTABLE_ALLOC_GROW(reallocated, newlen + 1, buf->alloc);
+ if (!reallocated)
-+ return -1;
++ return REFTABLE_OUT_OF_MEMORY_ERROR;
+ buf->buf = reallocated;
+ }
+
@@ reftable/basics.h: license that can be found in the LICENSE file or at
+};
+#define REFTABLE_BUF_INIT { 0 }
+
++/*
++ * Initialize the buffer such that it is ready for use. This is equivalent to
++ * using REFTABLE_BUF_INIT for stack-allocated variables.
++ */
+void reftable_buf_init(struct reftable_buf *buf);
++
++/*
++ * Release memory associated with the buffer. The buffer is reinitialized such
++ * that it can be reused for subsequent operations.
++ */
+void reftable_buf_release(struct reftable_buf *buf);
++
++/*
++ * Reset the buffer such that it is effectively empty, without releasing the
++ * memory that this structure holds on to. This is equivalent to calling
++ * `reftable_buf_setlen(buf, 0)`.
++ */
+void reftable_buf_reset(struct reftable_buf *buf);
++
++/*
++ * Trim the buffer to a shorter length by updating the `len` member and writing
++ * a NUL byte to `buf[len]`. Returns 0 on success, -1 when `len` points outside
++ * of the array.
++ */
+int reftable_buf_setlen(struct reftable_buf *buf, size_t len);
++
++/*
++ * Lexicographically compare the two buffers. Returns 0 when both buffers have
++ * the same contents, -1 when `a` is lexicographically smaller than `b`, and 1
++ * otherwise.
++ */
+int reftable_buf_cmp(const struct reftable_buf *a, const struct reftable_buf *b);
++
++/*
++ * Add the given bytes to the buffer. Returns 0 on success,
++ * REFTABLE_OUT_OF_MEMORY_ERROR on allocation failure.
++ */
+int reftable_buf_add(struct reftable_buf *buf, const void *data, size_t len);
++
++/* Equivalent to `reftable_buf_add(buf, s, strlen(s))`. */
+int reftable_buf_addstr(struct reftable_buf *buf, const char *s);
++
++/*
++ * Detach the buffer from the structure such that the underlying memory is now
++ * owned by the caller. The buffer is reinitialized such that it can be reused
++ * for subsequent operations.
++ */
+char *reftable_buf_detach(struct reftable_buf *buf);
+
/* Bigendian en/decoding of integers */ 4: e2ac27dbca0 ! 4: e1ff1af1f30 reftable: convert from strbuf to reftable_buf
@@ Commit message
reftable: convert from strbuf to reftable_buf
Convert the reftable library to use the reftable_buf interface instead
- of the strbuf interface. This is a mechanical change via sed(1) and
- does not yet handle allocation failures. These will be addressed in
- subsequent commits.
+ of the strbuf interface. This is mostly a mechanical change via sed(1)
+ with some manual fixes where functions for strbuf and reftable_buf
+ differ. The converted code does not yet handle allocation failures. This
+ will be handled in subsequent commits.
Signed-off-by Patrick Steinhardt <[email protected]>
5: da432d7a384 = 5: fe8c9ace463 reftable/blocksource: adapt interface name 6: 797e435ed2e = 6: 8c98745233a t/unit-tests: check for `reftable_buf` allocation errors 7: 66ee431db46 = 7: 1f08163009b reftable/stack: adapt `format_name()` to handle allocation failures 8: f5ae5ec27d6 = 8: 5798d76d7a4 reftable/record: adapt `reftable_record_key()` to handle allocation failures 9: d66d9e50e06 = 9: a9582d51dd1 reftable/stack: adapt `stack_filename()` to handle allocation failures 10: 8406948ae74 = 10: 90819c90f38 reftable: handle trivial `reftable_buf` errors
Signature · 1 line
2.47.0.dirty
PATCH v2 1/10 reftable: stop using `strbuf_addbuf()`
We're about to introduce our own reftable_buf type to replace strbuf. Get rid of the seldomly-used strbuf_addbuf() function such that we have to reimplement one less function.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/block.c | 2 +- reftable/record.c | 6 +++--- reftable/writer.c | 7 ++++--- 3 files changed, 8 insertions(+), 7 deletions(-)
reftable/block.c +1 -1
@@ -60,7 +60,7 @@ static int block_writer_register_restart(struct block_writer *w, int n, w->next += n;strbuf_reset (&w->last_key); -strbuf_addbuf (&w->last_key, key); +strbuf_add (&w->last_key, key->buf, key->len); w->entries++;return 0 ; }
reftable/record.c +3 -3
@@ -1031,7 +1031,7 @@ static void reftable_index_record_key(const void *r, struct strbuf *dest) {const struct reftable_index_record *rec = r;strbuf_reset (dest); -strbuf_addbuf (dest, &rec->last_key); +strbuf_add (dest, rec->last_key.buf, rec->last_key.len); } static int reftable_index_record_copy_from(void *rec, const void *src_rec,
@@ -1041,7 +1041,7 @@ static int reftable_index_record_copy_from(void *rec, const void *src_rec,const struct reftable_index_record *src = src_rec;strbuf_reset (&dst->last_key); -strbuf_addbuf (&dst->last_key, &src->last_key); +strbuf_add (&dst->last_key, src->last_key.buf, src->last_key.len); dst->offset = src->offset;return 0 ;
@@ -1085,7 +1085,7 @@ static int reftable_index_record_decode(void *rec, struct strbuf key,int n =0 ;strbuf_reset (&r->last_key); -strbuf_addbuf (&r->last_key, &key); +strbuf_add (&r->last_key, key.buf, key.len); n =get_var_int (&r->offset, &in);if (n <0 )
reftable/writer.c +4 -3
@@ -225,7 +225,7 @@ static int writer_index_hash(struct reftable_writer *w, struct strbuf *hash) *key = empty;strbuf_reset (&key->hash); -strbuf_addbuf (&key->hash, hash); +strbuf_add (&key->hash, hash->buf, hash->len);tree_insert (&w->obj_index_tree, key, &obj_index_tree_node_compare); }else {
@@ -256,7 +256,7 @@ static int writer_add_record(struct reftable_writer *w, }strbuf_reset (&w->last_key); -strbuf_addbuf (&w->last_key, &key); +strbuf_add (&w->last_key, key.buf, key.len);if (!w->block_writer) { err =writer_reinit_block_writer (w,reftable_record_type (rec));if (err <0 )
@@ -778,7 +778,8 @@ static int writer_flush_nonempty_block(struct reftable_writer *w) index_record.offset = w->next;strbuf_reset (&index_record.last_key); -strbuf_addbuf (&index_record.last_key, &w->block_writer->last_key); +strbuf_add (&index_record.last_key, w->block_writer->last_key.buf, + w->block_writer->last_key.len); w->index[w->index_len] = index_record; w->index_len++;
Signature · 1 line
2.47.0.dirty
PATCH v2 2/10 reftable: stop using `strbuf_addf()`
In reply to Patrick SteinhardtWe're about to introduce our own reftable_buf type to replace strbuf. Get rid of the seldomly-used strbuf_addf() function such that we have to reimplement one less function.
While at it, remove a useless call to strbuf_reset() in t_reftable_stack_auto_compaction_with_locked_tables(). We don't write to the buffer before this and initialize it with STRBUF_INIT, so there is no need to reset anything.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/stack.c | 18 ++++++++----- t/unit-tests/t-reftable-block.c | 7 +++-- t/unit-tests/t-reftable-readwrite.c | 20 +++++++------- t/unit-tests/t-reftable-stack.c | 42 ++++++++++++++++------------- 4 files changed, 50 insertions(+), 37 deletions(-)
reftable/stack.c +12 -6
@@ -1387,12 +1387,18 @@ static int stack_compact_range(struct reftable_stack *st,* have just written. In case the compacted table became empty we * simply skip writing it. */ -for (i =0 ; i < first_to_replace; i++) -strbuf_addf (&tables_list_buf,"%s\n" , names[i]); -if (!is_empty_table) -strbuf_addf (&tables_list_buf,"%s\n" , new_table_name.buf); -for (i = last_to_replace +1 ; names[i]; i++) -strbuf_addf (&tables_list_buf,"%s\n" , names[i]); +for (i =0 ; i < first_to_replace; i++) { +strbuf_addstr (&tables_list_buf, names[i]); +strbuf_addstr (&tables_list_buf,"\n" ); + } +if (!is_empty_table) { +strbuf_addstr (&tables_list_buf, new_table_name.buf); +strbuf_addstr (&tables_list_buf,"\n" ); + } +for (i = last_to_replace +1 ; names[i]; i++) { +strbuf_addstr (&tables_list_buf, names[i]); +strbuf_addstr (&tables_list_buf,"\n" ); + } err =write_in_full (get_lock_file_fd (&tables_list_lock), tables_list_buf.buf, tables_list_buf.len);
t/unit-tests/t-reftable-block.c +5 -2
@@ -308,10 +308,13 @@ static void t_index_block_read_write(void)check (!ret);for (i =0 ; i <N ; i++) { -strbuf_init (&recs[i].u.idx.last_key,9 ); +char buf[128 ]; + +snprintf (buf,sizeof (buf),"branch%02" PRIuMAX, (uintmax_t )i); +strbuf_init (&recs[i].u.idx.last_key,9 ); recs[i].type =BLOCK_TYPE_INDEX ; -strbuf_addf (&recs[i].u.idx.last_key,"branch%02" PRIuMAX, (uintmax_t )i); +strbuf_addstr (&recs[i].u.idx.last_key, buf); recs[i].u.idx.offset = i; ret =block_writer_add (&bw, &recs[i]);
t/unit-tests/t-reftable-readwrite.c +9 -11
@@ -753,12 +753,13 @@ static void t_write_multiple_indices(void)struct reftable_write_options opts = { .block_size =100 , }; -struct strbuf writer_buf =STRBUF_INIT , buf =STRBUF_INIT ; +struct strbuf writer_buf =STRBUF_INIT ;struct reftable_block_source source = {0 };struct reftable_iterator it = {0 };const struct reftable_stats *stats;struct reftable_writer *writer;struct reftable_reader *reader; +char buf[128 ];int err, i; writer =t_reftable_strbuf_writer (&writer_buf, &opts);
@@ -770,9 +771,8 @@ static void t_write_multiple_indices(void) .value.val1 = {i}, }; -strbuf_reset (&buf); -strbuf_addf (&buf,"refs/heads/%04d" , i); - ref.refname = buf.buf, +snprintf (buf,sizeof (buf),"refs/heads/%04d" , i); + ref.refname = buf; err =reftable_writer_add_ref (writer, &ref);check (!err);
@@ -788,9 +788,8 @@ static void t_write_multiple_indices(void) }, }; -strbuf_reset (&buf); -strbuf_addf (&buf,"refs/heads/%04d" , i); - log.refname = buf.buf, +snprintf (buf,sizeof (buf),"refs/heads/%04d" , i); + log.refname = buf; err =reftable_writer_add_log (writer, &log);check (!err);
@@ -824,7 +823,6 @@ static void t_write_multiple_indices(void)reftable_writer_free (writer);reftable_reader_decref (reader);strbuf_release (&writer_buf); -strbuf_release (&buf); }static void t_write_multi_level_index (void )
@@ -848,10 +846,10 @@ static void t_write_multi_level_index(void) .value_type = REFTABLE_REF_VAL1, .value.val1 = {i}, }; +char buf[128 ]; -strbuf_reset (&buf); -strbuf_addf (&buf,"refs/heads/%03" PRIuMAX, (uintmax_t )i); - ref.refname = buf.buf, +snprintf (buf,sizeof (buf),"refs/heads/%03" PRIuMAX, (uintmax_t )i); + ref.refname = buf; err =reftable_writer_add_ref (writer, &ref);check (!err);
t/unit-tests/t-reftable-stack.c +24 -18
110 lines not on this page · raw message
Signature · 1 line
2.47.0.dirty
PATCH v2 3/10 reftable/basics: provide new `reftable_buf` interface
In reply to Patrick SteinhardtImplement a new reftable_buf interface that will replace Git's own strbuf interface. This is done due to three reasons:
- The strbuf interfaces do not handle memory allocation failures and instead causes us to die. This is okay in the context of Git, but is not in the context of the reftable library, which is supposed to be usable by third-party applications.
- The strbuf interface is quite deeply tied into Git, which makes it hard to use the reftable library as a standalone library. Any dependent would have to carefully extract the relevant parts of it to make things work, which is not all that sensible.
- The strbuf interface does not use the pluggable allocators that can be set up via refatble_set_alloc().
So we have good reasons to use our own type, and the implementation is rather trivial. Implement our own type. Conversion of the reftable library will be handled in subsequent commits.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/basics.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++ reftable/basics.h | 56 +++++++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+)
reftable/basics.c +74 -0
@@ -9,6 +9,7 @@ license that can be found in the LICENSE file or at#define REFTABLE_ALLOW_BANNED_ALLOCATORS #include "basics.h" #include "reftable-basics.h" +#include "reftable-error.h" static void *(*reftable_malloc_ptr)(size_t sz);static void *(*reftable_realloc_ptr)(void *,size_t );
@@ -69,6 +70,79 @@ void reftable_set_alloc(void *(*malloc)(size_t), reftable_free_ptr = free; } +void reftable_buf_init (struct reftable_buf *buf) +{ +struct reftable_buf empty =REFTABLE_BUF_INIT ; + *buf = empty; +} + +void reftable_buf_release (struct reftable_buf *buf) +{ +reftable_free (buf->buf); +reftable_buf_init (buf); +} + +void reftable_buf_reset (struct reftable_buf *buf) +{ +if (buf->alloc) { + buf->len =0 ; + buf->buf[0 ] ='\0' ; + } +} + +int reftable_buf_setlen (struct reftable_buf *buf,size_t len) +{ +if (len > buf->len) +return -1 ; +if (len == buf->len) +return 0 ; + buf->buf[len] ='\0' ; + buf->len = len; +return 0 ; +} + +int reftable_buf_cmp (const struct reftable_buf *a,const struct reftable_buf *b) +{ +size_t len = a->len < b->len ? a->len : b->len; +if (len) { +int cmp =memcmp (a->buf, b->buf, len); +if (cmp) +return cmp; + } +return a->len < b->len ?-1 : a->len != b->len; +} + +int reftable_buf_add (struct reftable_buf *buf,const void *data,size_t len) +{ +size_t newlen = buf->len + len; + +if (newlen +1 > buf->alloc) { +char *reallocated = buf->buf; +REFTABLE_ALLOC_GROW (reallocated, newlen +1 , buf->alloc); +if (!reallocated) +return REFTABLE_OUT_OF_MEMORY_ERROR ; + buf->buf = reallocated; + } + +memcpy (buf->buf + buf->len, data, len); + buf->buf[newlen] ='\0' ; + buf->len = newlen; + +return 0 ; +} + +int reftable_buf_addstr (struct reftable_buf *buf,const char *s) +{ +return reftable_buf_add (buf, s,strlen (s)); +} + +char *reftable_buf_detach (struct reftable_buf *buf) +{ +char *result = buf->buf; +reftable_buf_init (buf); +return result; +} +void put_be24 (uint8_t *out,uint32_t i) { out[0 ] = (uint8_t )((i >>16 ) &0xff );
reftable/basics.h +56 -0
62 lines not on this page · raw message
Signature · 1 line
2.47.0.dirty
PATCH v2 4/10 reftable: convert from `strbuf` to `reftable_buf`
In reply to Patrick SteinhardtConvert the reftable library to use the reftable_buf interface instead of the strbuf interface. This is mostly a mechanical change via sed(1) with some manual fixes where functions for strbuf and reftable_buf differ. The converted code does not yet handle allocation failures. This will be handled in subsequent commits.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/basics.c | 2 +- reftable/basics.h | 3 +- reftable/block.c | 34 ++++---- reftable/block.h | 14 ++-- reftable/blocksource.c | 6 +- reftable/blocksource.h | 3 +- reftable/iter.c | 6 +- reftable/iter.h | 8 +- reftable/reader.c | 16 ++-- reftable/record.c | 80 +++++++++---------- reftable/record.h | 21 ++--- reftable/stack.c | 120 ++++++++++++++-------------- reftable/system.h | 1 - reftable/writer.c | 66 +++++++-------- reftable/writer.h | 2 +- t/unit-tests/lib-reftable.c | 4 +- t/unit-tests/lib-reftable.h | 7 +- t/unit-tests/t-reftable-basics.c | 16 ++-- t/unit-tests/t-reftable-block.c | 42 +++++----- t/unit-tests/t-reftable-merged.c | 26 +++--- t/unit-tests/t-reftable-reader.c | 8 +- t/unit-tests/t-reftable-readwrite.c | 92 ++++++++++----------- t/unit-tests/t-reftable-record.c | 74 ++++++++--------- t/unit-tests/t-reftable-stack.c | 90 ++++++++++----------- 24 files changed, 371 insertions(+), 370 deletions(-)
reftable/basics.c +1 -1
@@ -260,7 +260,7 @@ int names_equal(const char **a, const char **b)return a[i] == b[i]; } -int common_prefix_size (struct strbuf *a,struct strbuf *b) +int common_prefix_size (struct reftable_buf *a,struct reftable_buf *b) {int p =0 ;for (; p < a->len && p < b->len; p++) {
reftable/basics.h +1 -2
@@ -144,8 +144,7 @@ char *reftable_strdup(const char *str);#endif /* Find the longest shared prefix size of `a` and `b` */ -struct strbuf ; -int common_prefix_size (struct strbuf *a,struct strbuf *b); +int common_prefix_size (struct reftable_buf *a,struct reftable_buf *b);int hash_size (uint32_t id);
reftable/block.c +17 -17
@@ -38,7 +38,7 @@ int footer_size(int version) }static int block_writer_register_restart (struct block_writer *w,int n, -int is_restart,struct strbuf *key) +int is_restart,struct reftable_buf *key) {int rlen = w->restart_len;if (rlen >=MAX_RESTARTS ) {
@@ -59,8 +59,8 @@ static int block_writer_register_restart(struct block_writer *w, int n, w->next += n; -strbuf_reset (&w->last_key); -strbuf_add (&w->last_key, key->buf, key->len); +reftable_buf_reset (&w->last_key); +reftable_buf_add (&w->last_key, key->buf, key->len); w->entries++;return 0 ; }
@@ -98,8 +98,8 @@ uint8_t block_writer_type(struct block_writer *bw)empty key. */ int block_writer_add (struct block_writer *w,struct reftable_record *rec) { -struct strbuf empty =STRBUF_INIT ; -struct strbuf last = +struct reftable_buf empty =REFTABLE_BUF_INIT ; +struct reftable_buf last = w->entries % w->restart_interval ==0 ? empty : w->last_key;struct string_view out = { .buf = w->buf + w->next,
@@ -109,7 +109,7 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)struct string_view start = out;int is_restart =0 ; -struct strbuf key =STRBUF_INIT ; +struct reftable_buf key =REFTABLE_BUF_INIT ;int n =0 ;int err =-1 ;
@@ -133,7 +133,7 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec) err =block_writer_register_restart (w, start.len - out.len, is_restart, &key); done: -strbuf_release (&key); +reftable_buf_release (&key);return err; }
@@ -325,7 +325,7 @@ uint8_t block_reader_type(const struct block_reader *r)return r->block.data[r->header_off]; } -int block_reader_first_key (const struct block_reader *br,struct strbuf *key) +int block_reader_first_key (const struct block_reader *br,struct reftable_buf *key) {int off = br->header_off +4 , n;struct string_view in = {
@@ -334,7 +334,7 @@ int block_reader_first_key(const struct block_reader *br, struct strbuf *key) };uint8_t extra =0 ; -strbuf_reset (key); +reftable_buf_reset (key); n =reftable_decode_key (key, &extra, in);if (n <0 )
@@ -355,13 +355,13 @@ void block_iter_seek_start(struct block_iter *it, const struct block_reader *br) it->block = br->block.data; it->block_len = br->block_len; it->hash_size = br->hash_size; -strbuf_reset (&it->last_key); +reftable_buf_reset (&it->last_key); it->next_off = br->header_off +4 ; }struct restart_needle_less_args {int error; -struct strbuf needle; +struct reftable_buf needle;const struct block_reader *reader; };
@@ -433,7 +433,7 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec)void block_iter_reset (struct block_iter *it) { -strbuf_reset (&it->last_key); +reftable_buf_reset (&it->last_key); it->next_off =0 ; it->block =NULL ; it->block_len =0 ;
@@ -442,12 +442,12 @@ void block_iter_reset(struct block_iter *it)void block_iter_close (struct block_iter *it) { -strbuf_release (&it->last_key); -strbuf_release (&it->scratch); +reftable_buf_release (&it->last_key); +reftable_buf_release (&it->scratch); }int block_iter_seek_key (struct block_iter *it,const struct block_reader *br, -struct strbuf *want) +struct reftable_buf *want) {struct restart_needle_less_args args = { .needle = *want,
@@ -537,7 +537,7 @@ int block_iter_seek_key(struct block_iter *it, const struct block_reader *br,* with themselves. */ reftable_record_key (&rec, &it->last_key); -if (strbuf_cmp (&it->last_key, want) >=0 ) { +if (reftable_buf_cmp (&it->last_key, want) >=0 ) { it->next_off = prev_off; goto done; }
@@ -554,7 +554,7 @@ void block_writer_release(struct block_writer *bw)REFTABLE_FREE_AND_NULL (bw->zstream);REFTABLE_FREE_AND_NULL (bw->restarts);REFTABLE_FREE_AND_NULL (bw->compressed); -strbuf_release (&bw->last_key); +reftable_buf_release (&bw->last_key);/* the block is not owned. */ }
reftable/block.h +7 -7
@@ -38,7 +38,7 @@ struct block_writer {uint32_t restart_len;uint32_t restart_cap; -struct strbuf last_key; +struct reftable_buf last_key;int entries; };
@@ -98,7 +98,7 @@ void block_reader_release(struct block_reader *br);uint8_t block_reader_type (const struct block_reader *r);/* Decodes the first key in the block */ -int block_reader_first_key (const struct block_reader *br,struct strbuf *key); +int block_reader_first_key (const struct block_reader *br,struct reftable_buf *key);/* Iterate over entries in a block */ struct block_iter {
@@ -109,13 +109,13 @@ struct block_iter {int hash_size;/* key for last entry we read. */ -struct strbuf last_key; -struct strbuf scratch; +struct reftable_buf last_key; +struct reftable_buf scratch; };#define BLOCK_ITER_INIT { \ - .last_key = STRBUF_INIT, \ - .scratch = STRBUF_INIT, \ + .last_key = REFTABLE_BUF_INIT, \ + .scratch = REFTABLE_BUF_INIT, \ }/* Position `it` at start of the block */
@@ -123,7 +123,7 @@ void block_iter_seek_start(struct block_iter *it, const struct block_reader *br)/* Position `it` to the `want` key in the block */ int block_iter_seek_key (struct block_iter *it,const struct block_reader *br, -struct strbuf *want); +struct reftable_buf *want);/* return < 0 for error, 0 for OK, > 0 for EOF. */ int block_iter_next (struct block_iter *it,struct reftable_record *rec);
reftable/blocksource.c +3 -3
@@ -27,7 +27,7 @@ static void strbuf_close(void *b UNUSED)static int strbuf_read_block (void *v,struct reftable_block *dest,uint64_t off,uint32_t size) { -struct strbuf *b = v; +struct reftable_buf *b = v;assert (off + size <= b->len);REFTABLE_CALLOC_ARRAY (dest->data, size);if (!dest->data)
@@ -39,7 +39,7 @@ static int strbuf_read_block(void *v, struct reftable_block *dest, uint64_t off,static uint64_t strbuf_size (void *b) { -return ((struct strbuf *)b)->len; +return ((struct reftable_buf *)b)->len; }static struct reftable_block_source_vtable strbuf_vtable = {
@@ -50,7 +50,7 @@ static struct reftable_block_source_vtable strbuf_vtable = { };void block_source_from_strbuf (struct reftable_block_source *bs, -struct strbuf *buf) +struct reftable_buf *buf) {assert (!bs->ops); bs->ops = &strbuf_vtable;
reftable/blocksource.h +2 -1
@@ -12,9 +12,10 @@ license that can be found in the LICENSE file or at#include "system.h" struct reftable_block_source ; +struct reftable_buf ;/* Create an in-memory block source for reading reftables */ void block_source_from_strbuf (struct reftable_block_source *bs, -struct strbuf *buf); +struct reftable_buf *buf);#endif
reftable/iter.c +3 -3
@@ -55,7 +55,7 @@ void iterator_set_empty(struct reftable_iterator *it)static void filtering_ref_iterator_close (void *iter_arg) {struct filtering_ref_iterator *fri = iter_arg; -strbuf_release (&fri->oid); +reftable_buf_release (&fri->oid);reftable_iterator_destroy (&fri->it); }
@@ -115,7 +115,7 @@ static void indexed_table_ref_iter_close(void *p)block_iter_close (&it->cur);reftable_block_done (&it->block_reader.block);reftable_free (it->offsets); -strbuf_release (&it->oid); +reftable_buf_release (&it->oid); }static int indexed_table_ref_iter_next_block (struct indexed_table_ref_iter *it)
@@ -197,7 +197,7 @@ int indexed_table_ref_iter_new(struct indexed_table_ref_iter **dest, *itr = empty; itr->r = r; -strbuf_add (&itr->oid, oid, oid_len); +reftable_buf_add (&itr->oid, oid, oid_len); itr->offsets = offsets; itr->offset_len = offset_len;
reftable/iter.h +4 -4
@@ -44,12 +44,12 @@ void iterator_set_empty(struct reftable_iterator *it);/* iterator that produces only ref records that point to `oid` */ struct filtering_ref_iterator { -struct strbuf oid; +struct reftable_buf oid;struct reftable_iterator it; };#define FILTERING_REF_ITERATOR_INIT \ { \ - .oid = STRBUF_INIT \ + .oid = REFTABLE_BUF_INIT \ } void iterator_from_filtering_ref_iterator(struct reftable_iterator *,
@@ -60,7 +60,7 @@ void iterator_from_filtering_ref_iterator(struct reftable_iterator *,*/ struct indexed_table_ref_iter {struct reftable_reader *r; -struct strbuf oid; +struct reftable_buf oid;/* mutable */ uint64_t *offsets;
@@ -75,7 +75,7 @@ struct indexed_table_ref_iter {#define INDEXED_TABLE_REF_ITER_INIT { \ .cur = BLOCK_ITER_INIT, \ - .oid = STRBUF_INIT, \ + .oid = REFTABLE_BUF_INIT, \ } void iterator_from_indexed_table_ref_iter(struct reftable_iterator *it,
reftable/reader.c +8 -8
@@ -350,8 +350,8 @@ static int table_iter_seek_start(struct table_iter *ti, uint8_t typ, int index)static int table_iter_seek_linear (struct table_iter *ti,struct reftable_record *want) { -struct strbuf want_key =STRBUF_INIT ; -struct strbuf got_key =STRBUF_INIT ; +struct reftable_buf want_key =REFTABLE_BUF_INIT ; +struct reftable_buf got_key =REFTABLE_BUF_INIT ;struct reftable_record rec;int err;
@@ -401,7 +401,7 @@ static int table_iter_seek_linear(struct table_iter *ti,if (err <0 ) goto done; -if (strbuf_cmp (&got_key, &want_key) >0 ) { +if (reftable_buf_cmp (&got_key, &want_key) >0 ) {table_iter_block_done (&next);break ; }
@@ -422,8 +422,8 @@ static int table_iter_seek_linear(struct table_iter *ti, done:reftable_record_release (&rec); -strbuf_release (&want_key); -strbuf_release (&got_key); +reftable_buf_release (&want_key); +reftable_buf_release (&got_key);return err; }
@@ -431,11 +431,11 @@ static int table_iter_seek_indexed(struct table_iter *ti, struct reftable_record *rec) {struct reftable_record want_index = { - .type =BLOCK_TYPE_INDEX , .u.idx = { .last_key =STRBUF_INIT } + .type =BLOCK_TYPE_INDEX , .u.idx = { .last_key =REFTABLE_BUF_INIT } };struct reftable_record index_result = { .type =BLOCK_TYPE_INDEX , - .u.idx = { .last_key =STRBUF_INIT }, + .u.idx = { .last_key =REFTABLE_BUF_INIT }, };int err;
@@ -765,7 +765,7 @@ static int reftable_reader_refs_for_unindexed(struct reftable_reader *r, } *filter = empty; -strbuf_add (&filter->oid, oid, oid_len); +reftable_buf_add (&filter->oid, oid, oid_len);iterator_from_table_iter (&filter->it, ti);iterator_from_filtering_ref_iterator (it, filter);
reftable/record.c +40 -40
235 lines not on this page · raw message
reftable/record.h +11 -10
@@ -9,6 +9,7 @@ license that can be found in the LICENSE file or at#ifndef RECORD_H #define RECORD_H +#include "basics.h" #include "system.h" #include <stdint.h>
@@ -38,8 +39,8 @@ int put_var_int(struct string_view *dest, uint64_t val);/* Methods for records. */ struct reftable_record_vtable { -/* encode the key of to a uint8_t strbuf. */ -void (*key)(const void *rec,struct strbuf *dest); +/* encode the key of to a uint8_t reftable_buf. */ +void (*key)(const void *rec,struct reftable_buf *dest);/* The record type of ('r' for ref). */ uint8_t type;
@@ -54,9 +55,9 @@ struct reftable_record_vtable {int (*encode)(const void *rec,struct string_view dest,int hash_size);/* decode data from `src` into the record. */ -int (*decode)(void *rec,struct strbuf key,uint8_t extra, +int (*decode)(void *rec,struct reftable_buf key,uint8_t extra,struct string_view src,int hash_size, -struct strbuf *scratch); +struct reftable_buf *scratch);/* deallocate and null the record. */ void (*release)(void *rec);
@@ -83,7 +84,7 @@ int reftable_is_block_type(uint8_t typ);/* Encode `key` into `dest`. Sets `is_restart` to indicate a restart. Returns * number of bytes written. */ int reftable_encode_key (int *is_restart,struct string_view dest, -struct strbuf prev_key,struct strbuf key, +struct reftable_buf prev_key,struct reftable_buf key,uint8_t extra);/* Decode a record's key lengths. */
@@ -96,13 +97,13 @@ int reftable_decode_keylen(struct string_view in,* Decode into `last_key` and `extra` from `in`. `last_key` is expected to * contain the decoded key of the preceding record, if any. */ -int reftable_decode_key (struct strbuf *last_key,uint8_t *extra, +int reftable_decode_key (struct reftable_buf *last_key,uint8_t *extra,struct string_view in);/* reftable_index_record are used internally to speed up lookups. */ struct reftable_index_record {uint64_t offset;/* Offset of block */ -struct strbuf last_key;/* Last key of the block. */ +struct reftable_buf last_key;/* Last key of the block. */ };/* reftable_obj_record stores an object ID => ref mapping. */
@@ -136,15 +137,15 @@ void reftable_record_init(struct reftable_record *rec, uint8_t typ);/* see struct record_vtable */ int reftable_record_cmp (struct reftable_record *a,struct reftable_record *b);int reftable_record_equal (struct reftable_record *a,struct reftable_record *b,int hash_size); -void reftable_record_key (struct reftable_record *rec,struct strbuf *dest); +void reftable_record_key (struct reftable_record *rec,struct reftable_buf *dest);int reftable_record_copy_from (struct reftable_record *rec,struct reftable_record *src,int hash_size);uint8_t reftable_record_val_type (struct reftable_record *rec);int reftable_record_encode (struct reftable_record *rec,struct string_view dest,int hash_size); -int reftable_record_decode (struct reftable_record *rec,struct strbuf key, +int reftable_record_decode (struct reftable_record *rec,struct reftable_buf key,uint8_t extra,struct string_view src, -int hash_size,struct strbuf *scratch); +int hash_size,struct reftable_buf *scratch);int reftable_record_is_deletion (struct reftable_record *rec);static inline uint8_t reftable_record_type (struct reftable_record *rec)
reftable/stack.c +60 -60
305 lines not on this page · raw message
reftable/system.h +0 -1
@@ -13,7 +13,6 @@ license that can be found in the LICENSE file or at#include "git-compat-util.h" #include "lockfile.h" -#include "strbuf.h" #include "tempfile.h" #include "hash.h" /* hash ID, sizes.*/ #include "dir.h" /* remove_dir_recursively, for tests.*/
reftable/writer.c +33 -33
219 lines not on this page · raw message
reftable/writer.h +1 -1
@@ -19,7 +19,7 @@ struct reftable_writer {int (*flush)(void *);void *write_arg;int pending_padding; -struct strbuf last_key; +struct reftable_buf last_key;/* offset of next block to write. */ uint64_t next;
t/unit-tests/lib-reftable.c +2 -2
@@ -19,7 +19,7 @@ static int strbuf_writer_flush(void *arg UNUSED)return 0 ; } -struct reftable_writer *t_reftable_strbuf_writer (struct strbuf *buf, +struct reftable_writer *t_reftable_strbuf_writer (struct reftable_buf *buf,struct reftable_write_options *opts) {struct reftable_writer *writer;
@@ -29,7 +29,7 @@ struct reftable_writer *t_reftable_strbuf_writer(struct strbuf *buf,return writer; } -void t_reftable_write_to_buf(struct strbuf *buf, +void t_reftable_write_to_buf(struct reftable_buf *buf, struct reftable_ref_record *refs, size_t nrefs, struct reftable_log_record *logs,
t/unit-tests/lib-reftable.h +4 -3
@@ -2,15 +2,16 @@#define LIB_REFTABLE_H #include "git-compat-util.h" -#include "strbuf.h" #include "reftable/reftable-writer.h" +struct reftable_buf ; +void t_reftable_set_hash (uint8_t *p,int i,uint32_t id); -struct reftable_writer *t_reftable_strbuf_writer (struct strbuf *buf, +struct reftable_writer *t_reftable_strbuf_writer (struct reftable_buf *buf,struct reftable_write_options *opts); -void t_reftable_write_to_buf(struct strbuf *buf, +void t_reftable_write_to_buf(struct reftable_buf *buf, struct reftable_ref_record *refs, size_t nrecords, struct reftable_log_record *logs,
t/unit-tests/t-reftable-basics.c +8 -8
@@ -99,8 +99,8 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED) } if_test ("common_prefix_size works") { -struct strbuf a =STRBUF_INIT ; -struct strbuf b =STRBUF_INIT ; +struct reftable_buf a =REFTABLE_BUF_INIT ; +struct reftable_buf b =REFTABLE_BUF_INIT ;struct {const char *a, *b;int want;
@@ -113,14 +113,14 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED) };for (size_t i =0 ; i <ARRAY_SIZE (cases); i++) { -strbuf_addstr (&a, cases[i].a); -strbuf_addstr (&b, cases[i].b); +reftable_buf_addstr (&a, cases[i].a); +reftable_buf_addstr (&b, cases[i].b); check_int(common_prefix_size(&a, &b), ==, cases[i].want); -strbuf_reset (&a); -strbuf_reset (&b); +reftable_buf_reset (&a); +reftable_buf_reset (&b); } -strbuf_release (&a); -strbuf_release (&b); +reftable_buf_release (&a); +reftable_buf_release (&b); } if_test ("put_be24 and get_be24 work") {
t/unit-tests/t-reftable-block.c +21 -21
129 lines not on this page · raw message
t/unit-tests/t-reftable-merged.c +13 -13
@@ -20,7 +20,7 @@ static struct reftable_merged_table * merged_table_from_records(struct reftable_ref_record **refs, struct reftable_block_source **source, struct reftable_reader ***readers, const size_t *sizes, - struct strbuf *buf, const size_t n) + struct reftable_buf *buf, const size_t n) { struct reftable_merged_table *mt = NULL; struct reftable_write_options opts = {
@@ -75,7 +75,7 @@ static void t_merged_single_record(void)struct reftable_ref_record *refs[] = { r1, r2, r3 };size_t sizes[] = {ARRAY_SIZE (r1),ARRAY_SIZE (r2),ARRAY_SIZE (r3) }; -struct strbuf bufs[3 ] = {STRBUF_INIT ,STRBUF_INIT ,STRBUF_INIT }; +struct reftable_buf bufs[3 ] = {REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT };struct reftable_block_source *bs =NULL ;struct reftable_reader **readers =NULL ; struct reftable_merged_table *mt =
@@ -97,7 +97,7 @@ static void t_merged_single_record(void)readers_destroy (readers,3 );reftable_merged_table_free (mt);for (size_t i =0 ; i <ARRAY_SIZE (bufs); i++) -strbuf_release (&bufs[i]); +reftable_buf_release (&bufs[i]);reftable_free (bs); }
@@ -152,7 +152,7 @@ static void t_merged_refs(void)struct reftable_ref_record *refs[] = { r1, r2, r3 };size_t sizes[3 ] = {ARRAY_SIZE (r1),ARRAY_SIZE (r2),ARRAY_SIZE (r3) }; -struct strbuf bufs[3 ] = {STRBUF_INIT ,STRBUF_INIT ,STRBUF_INIT }; +struct reftable_buf bufs[3 ] = {REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT };struct reftable_block_source *bs =NULL ;struct reftable_reader **readers =NULL ; struct reftable_merged_table *mt =
@@ -192,7 +192,7 @@ static void t_merged_refs(void)reftable_free (out);for (i =0 ; i <3 ; i++) -strbuf_release (&bufs[i]); +reftable_buf_release (&bufs[i]);readers_destroy (readers,3 );reftable_merged_table_free (mt);reftable_free (bs);
@@ -234,8 +234,8 @@ static void t_merged_seek_multiple_times(void)size_t sizes[] = {ARRAY_SIZE (r1),ARRAY_SIZE (r2), }; -struct strbuf bufs[] = { -STRBUF_INIT ,STRBUF_INIT , +struct reftable_buf bufs[] = { +REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT , };struct reftable_block_source *sources =NULL ;struct reftable_reader **readers =NULL ;
@@ -265,7 +265,7 @@ static void t_merged_seek_multiple_times(void) }for (size_t i =0 ; i <ARRAY_SIZE (bufs); i++) -strbuf_release (&bufs[i]); +reftable_buf_release (&bufs[i]);readers_destroy (readers,ARRAY_SIZE (refs));reftable_ref_record_release (&rec);reftable_iterator_destroy (&it);
@@ -277,7 +277,7 @@ static struct reftable_merged_table * merged_table_from_log_records(struct reftable_log_record **logs, struct reftable_block_source **source, struct reftable_reader ***readers, const size_t *sizes, - struct strbuf *buf, const size_t n) + struct reftable_buf *buf, const size_t n) { struct reftable_merged_table *mt = NULL; struct reftable_write_options opts = {
@@ -361,7 +361,7 @@ static void t_merged_logs(void)struct reftable_log_record *logs[] = { r1, r2, r3 };size_t sizes[3 ] = {ARRAY_SIZE (r1),ARRAY_SIZE (r2),ARRAY_SIZE (r3) }; -struct strbuf bufs[3 ] = {STRBUF_INIT ,STRBUF_INIT ,STRBUF_INIT }; +struct reftable_buf bufs[3 ] = {REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT };struct reftable_block_source *bs =NULL ;struct reftable_reader **readers =NULL ; struct reftable_merged_table *mt = merged_table_from_log_records(
@@ -412,7 +412,7 @@ static void t_merged_logs(void)reftable_free (out);for (i =0 ; i <3 ; i++) -strbuf_release (&bufs[i]); +reftable_buf_release (&bufs[i]);readers_destroy (readers,3 );reftable_merged_table_free (mt);reftable_free (bs);
@@ -421,7 +421,7 @@ static void t_merged_logs(void)static void t_default_write_opts (void ) {struct reftable_write_options opts = {0 }; -struct strbuf buf =STRBUF_INIT ; +struct reftable_buf buf =REFTABLE_BUF_INIT ;struct reftable_writer *w =t_reftable_strbuf_writer (&buf, &opts);struct reftable_ref_record rec = { .refname = (char *)"master" ,
@@ -457,7 +457,7 @@ static void t_default_write_opts(void)reftable_reader_decref (rd);reftable_merged_table_free (merged); -strbuf_release (&buf); +reftable_buf_release (&buf); }
t/unit-tests/t-reftable-reader.c +4 -4
@@ -16,7 +16,7 @@ static int t_reader_seek_once(void)struct reftable_ref_record ref = {0 };struct reftable_iterator it = {0 };struct reftable_reader *reader; -struct strbuf buf =STRBUF_INIT ; +struct reftable_buf buf =REFTABLE_BUF_INIT ;int ret;t_reftable_write_to_buf (&buf, records,ARRAY_SIZE (records),NULL ,0 ,NULL );
@@ -40,7 +40,7 @@ static int t_reader_seek_once(void)reftable_ref_record_release (&ref);reftable_iterator_destroy (&it);reftable_reader_decref (reader); -strbuf_release (&buf); +reftable_buf_release (&buf);return 0 ; }
@@ -57,7 +57,7 @@ static int t_reader_reseek(void)struct reftable_ref_record ref = {0 };struct reftable_iterator it = {0 };struct reftable_reader *reader; -struct strbuf buf =STRBUF_INIT ; +struct reftable_buf buf =REFTABLE_BUF_INIT ;int ret;t_reftable_write_to_buf (&buf, records,ARRAY_SIZE (records),NULL ,0 ,NULL );
@@ -84,7 +84,7 @@ static int t_reader_reseek(void)reftable_ref_record_release (&ref);reftable_iterator_destroy (&it);reftable_reader_decref (reader); -strbuf_release (&buf); +reftable_buf_release (&buf);return 0 ; }
t/unit-tests/t-reftable-readwrite.c +46 -46
328 lines not on this page · raw message
t/unit-tests/t-reftable-record.c +37 -37
201 lines not on this page · raw message
t/unit-tests/t-reftable-stack.c +45 -45
214 lines not on this page · raw message
Signature · 1 line
2.47.0.dirty
PATCH v2 5/10 reftable/blocksource: adapt interface name
In reply to Patrick SteinhardtAdapt the name of the strbuf block source to no longer relate to this interface, but instead to the reftable_buf interface.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/blocksource.c | 26 +++++++++++++------------- reftable/blocksource.h | 4 ++-- t/unit-tests/t-reftable-block.c | 8 ++++---- t/unit-tests/t-reftable-merged.c | 6 +++--- t/unit-tests/t-reftable-reader.c | 4 ++-- t/unit-tests/t-reftable-readwrite.c | 24 ++++++++++++------------ 6 files changed, 36 insertions(+), 36 deletions(-)
reftable/blocksource.c +13 -13
@@ -13,19 +13,19 @@ license that can be found in the LICENSE file or at#include "reftable-blocksource.h" #include "reftable-error.h" -static void strbuf_return_block(void *b UNUSED, struct reftable_block *dest) +static void reftable_buf_return_block(void *b UNUSED, struct reftable_block *dest) {if (dest->len)memset (dest->data,0xff , dest->len);reftable_free (dest->data); } -static void strbuf_close(void *b UNUSED) +static void reftable_buf_close(void *b UNUSED) { } -static int strbuf_read_block (void *v,struct reftable_block *dest,uint64_t off, -uint32_t size) +static int reftable_buf_read_block (void *v,struct reftable_block *dest, +uint64_t off,uint32_t size) {struct reftable_buf *b = v;assert (off + size <= b->len);
@@ -37,23 +37,23 @@ static int strbuf_read_block(void *v, struct reftable_block *dest, uint64_t off,return size; } -static uint64_t strbuf_size (void *b) +static uint64_t reftable_buf_size (void *b) {return ((struct reftable_buf *)b)->len; } -static struct reftable_block_source_vtable strbuf_vtable = { - .size = &strbuf_size, - .read_block = &strbuf_read_block, - .return_block = &strbuf_return_block, - .close = &strbuf_close, +static struct reftable_block_source_vtable reftable_buf_vtable = { + .size = &reftable_buf_size, + .read_block = &reftable_buf_read_block, + .return_block = &reftable_buf_return_block, + .close = &reftable_buf_close, }; -void block_source_from_strbuf (struct reftable_block_source *bs, -struct reftable_buf *buf) +void block_source_from_buf (struct reftable_block_source *bs, +struct reftable_buf *buf) {assert (!bs->ops); - bs->ops = &strbuf_vtable; + bs->ops = &reftable_buf_vtable; bs->arg = buf; }
reftable/blocksource.h +2 -2
@@ -15,7 +15,7 @@ struct reftable_block_source;struct reftable_buf ;/* Create an in-memory block source for reading reftables */ -void block_source_from_strbuf (struct reftable_block_source *bs, -struct reftable_buf *buf); +void block_source_from_buf (struct reftable_block_source *bs, +struct reftable_buf *buf);#endif
t/unit-tests/t-reftable-block.c +4 -4
@@ -34,7 +34,7 @@ static void t_ref_block_read_write(void)REFTABLE_CALLOC_ARRAY (block.data, block_size);check (block.data !=NULL ); block.len = block_size; -block_source_from_strbuf (&block.source ,&buf); +block_source_from_buf (&block.source ,&buf); ret =block_writer_init (&bw,BLOCK_TYPE_REF , block.data, block_size, header_off,hash_size (GIT_SHA1_FORMAT_ID ));check (!ret);
@@ -128,7 +128,7 @@ static void t_log_block_read_write(void)REFTABLE_CALLOC_ARRAY (block.data, block_size);check (block.data !=NULL ); block.len = block_size; -block_source_from_strbuf (&block.source ,&buf); +block_source_from_buf (&block.source ,&buf); ret =block_writer_init (&bw,BLOCK_TYPE_LOG , block.data, block_size, header_off,hash_size (GIT_SHA1_FORMAT_ID ));check (!ret);
@@ -218,7 +218,7 @@ static void t_obj_block_read_write(void)REFTABLE_CALLOC_ARRAY (block.data, block_size);check (block.data !=NULL ); block.len = block_size; -block_source_from_strbuf (&block.source, &buf); +block_source_from_buf (&block.source, &buf); ret =block_writer_init (&bw,BLOCK_TYPE_OBJ , block.data, block_size, header_off,hash_size (GIT_SHA1_FORMAT_ID ));check (!ret);
@@ -302,7 +302,7 @@ static void t_index_block_read_write(void)REFTABLE_CALLOC_ARRAY (block.data, block_size);check (block.data !=NULL ); block.len = block_size; -block_source_from_strbuf (&block.source, &buf); +block_source_from_buf (&block.source, &buf); ret =block_writer_init (&bw,BLOCK_TYPE_INDEX , block.data, block_size, header_off,hash_size (GIT_SHA1_FORMAT_ID ));check (!ret);
t/unit-tests/t-reftable-merged.c +3 -3
@@ -35,7 +35,7 @@ merged_table_from_records(struct reftable_ref_record **refs,for (size_t i =0 ; i < n; i++) {t_reftable_write_to_buf (&buf[i], refs[i], sizes[i],NULL ,0 , &opts); -block_source_from_strbuf (&(*source)[i], &buf[i]); +block_source_from_buf (&(*source)[i], &buf[i]); err =reftable_reader_new (&(*readers)[i], &(*source)[i],"name" );
@@ -293,7 +293,7 @@ merged_table_from_log_records(struct reftable_log_record **logs,for (size_t i =0 ; i < n; i++) {t_reftable_write_to_buf (&buf[i],NULL ,0 , logs[i], sizes[i], &opts); -block_source_from_strbuf (&(*source)[i], &buf[i]); +block_source_from_buf (&(*source)[i], &buf[i]); err =reftable_reader_new (&(*readers)[i], &(*source)[i],"name" );
@@ -442,7 +442,7 @@ static void t_default_write_opts(void)check (!err);reftable_writer_free (w); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&rd, &source,"filename" );check (!err);
t/unit-tests/t-reftable-reader.c +2 -2
@@ -20,7 +20,7 @@ static int t_reader_seek_once(void)int ret;t_reftable_write_to_buf (&buf, records,ARRAY_SIZE (records),NULL ,0 ,NULL ); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); ret =reftable_reader_new (&reader, &source,"name" );check (!ret);
@@ -61,7 +61,7 @@ static int t_reader_reseek(void)int ret;t_reftable_write_to_buf (&buf, records,ARRAY_SIZE (records),NULL ,0 ,NULL ); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); ret =reftable_reader_new (&reader, &source,"name" );check (!ret);
t/unit-tests/t-reftable-readwrite.c +12 -12
@@ -24,7 +24,7 @@ static void t_buffer(void)int n;uint8_t in[] ="hello" ;reftable_buf_add (&buf, in,sizeof (in)); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); check_int(block_source_size(&source), ==, 6); n =block_source_read_block (&source, &out,0 ,sizeof (in)); check_int(n, ==, sizeof(in));
@@ -207,7 +207,7 @@ static void t_log_write_read(void)reftable_writer_free (w); w =NULL ; -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.log" );check (!err);
@@ -298,7 +298,7 @@ static void t_log_zlib_corruption(void)/* corrupt the data. */ buf.buf[50 ] ^=0x99 ; -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.log" );check (!err);
@@ -328,7 +328,7 @@ static void t_table_read_write_sequential(void)write_table (&names, &buf,N ,256 ,GIT_SHA1_FORMAT_ID ); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.ref" );check (!err);
@@ -380,7 +380,7 @@ static void t_table_read_api(void)write_table (&names, &buf,N ,256 ,GIT_SHA1_FORMAT_ID ); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.ref" );check (!err);
@@ -416,7 +416,7 @@ static void t_table_read_write_seek(int index, int hash_id)write_table (&names, &buf,N ,256 , hash_id); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.ref" );check (!err);
@@ -538,7 +538,7 @@ static void t_table_refs_for(int indexed)reftable_writer_free (w); w =NULL ; -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.ref" );check (!err);
@@ -600,7 +600,7 @@ static void t_write_empty_table(void) check_int(buf.len, ==, header_size(1) + footer_size(1)); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&rd, &source,"filename" );check (!err);
@@ -806,7 +806,7 @@ static void t_write_multiple_indices(void) check_int(stats->obj_stats.index_offset, >, 0); check_int(stats->log_stats.index_offset, >, 0); -block_source_from_strbuf (&source, &writer_buf); +block_source_from_buf (&source, &writer_buf); err =reftable_reader_new (&reader, &source,"filename" );check (!err);
@@ -863,7 +863,7 @@ static void t_write_multi_level_index(void) stats =reftable_writer_stats (writer); check_int(stats->ref_stats.max_index_level, ==, 2); -block_source_from_strbuf (&source, &writer_buf); +block_source_from_buf (&source, &writer_buf); err =reftable_reader_new (&reader, &source,"filename" );check (!err);
@@ -889,7 +889,7 @@ static void t_corrupt_table_empty(void)struct reftable_reader *reader;int err; -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.log" ); check_int(err, ==, REFTABLE_FORMAT_ERROR); }
@@ -903,7 +903,7 @@ static void t_corrupt_table(void)int err;reftable_buf_add (&buf, zeros,sizeof (zeros)); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.log" ); check_int(err, ==, REFTABLE_FORMAT_ERROR);
Signature · 1 line
2.47.0.dirty
PATCH v2 6/10 t/unit-tests: check for `reftable_buf` allocation errors
In reply to Patrick SteinhardtAdapt our unit tests to check for allocations errors.
Signed-off-by Patrick Steinhardt <[email protected]>
--- t/unit-tests/t-reftable-basics.c | 4 +- t/unit-tests/t-reftable-block.c | 4 +- t/unit-tests/t-reftable-readwrite.c | 8 ++-- t/unit-tests/t-reftable-record.c | 14 +++---- t/unit-tests/t-reftable-stack.c | 58 ++++++++++++++--------------- 5 files changed, 44 insertions(+), 44 deletions(-)
t/unit-tests/t-reftable-basics.c +2 -2
@@ -113,8 +113,8 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED) };for (size_t i =0 ; i <ARRAY_SIZE (cases); i++) { -reftable_buf_addstr (&a, cases[i].a); -reftable_buf_addstr (&b, cases[i].b); +check (!reftable_buf_addstr (&a, cases[i].a)); +check (!reftable_buf_addstr (&b, cases[i].b)); check_int(common_prefix_size(&a, &b), ==, cases[i].want);reftable_buf_reset (&a);reftable_buf_reset (&b);
t/unit-tests/t-reftable-block.c +2 -2
@@ -167,7 +167,7 @@ static void t_log_block_read_write(void)for (i =0 ; i <N ; i++) {block_iter_reset (&it);reftable_buf_reset (&want); -reftable_buf_addstr (&want, recs[i].u.log.refname); +check (!reftable_buf_addstr (&want, recs[i].u.log.refname)); ret =block_iter_seek_key (&it, &br, &want); check_int(ret, ==, 0);
@@ -314,7 +314,7 @@ static void t_index_block_read_write(void)reftable_buf_init (&recs[i].u.idx.last_key); recs[i].type =BLOCK_TYPE_INDEX ; -reftable_buf_addstr (&recs[i].u.idx.last_key, buf); +check (!reftable_buf_addstr (&recs[i].u.idx.last_key, buf)); recs[i].u.idx.offset = i; ret =block_writer_add (&bw, &recs[i]);
t/unit-tests/t-reftable-readwrite.c +4 -4
@@ -23,7 +23,7 @@ static void t_buffer(void)struct reftable_block out = {0 };int n;uint8_t in[] ="hello" ; -reftable_buf_add (&buf, in,sizeof (in)); +check (!reftable_buf_add (&buf, in,sizeof (in)));block_source_from_buf (&source, &buf); check_int(block_source_size(&source), ==, 6); n =block_source_read_block (&source, &out,0 ,sizeof (in));
@@ -443,8 +443,8 @@ static void t_table_read_write_seek(int index, int hash_id)reftable_iterator_destroy (&it); } -reftable_buf_addstr (&pastLast, names[N -1 ]); -reftable_buf_addstr (&pastLast,"/" ); +check (!reftable_buf_addstr (&pastLast, names[N -1 ])); +check (!reftable_buf_addstr (&pastLast,"/" )); err =reftable_reader_init_ref_iterator (reader, &it);check (!err);
@@ -901,7 +901,7 @@ static void t_corrupt_table(void)struct reftable_block_source source = {0 };struct reftable_reader *reader;int err; -reftable_buf_add (&buf, zeros,sizeof (zeros)); +check (!reftable_buf_add (&buf, zeros,sizeof (zeros)));block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.log" );
t/unit-tests/t-reftable-record.c +7 -7
@@ -335,14 +335,14 @@ static void t_key_roundtrip(void)int n, m;uint8_t rt_extra; -reftable_buf_addstr (&last_key,"refs/heads/master" ); -reftable_buf_addstr (&key,"refs/tags/bla" ); +check (!reftable_buf_addstr (&last_key,"refs/heads/master" )); +check (!reftable_buf_addstr (&key,"refs/tags/bla" )); extra =6 ; n =reftable_encode_key (&restart, dest, last_key, key, extra);check (!restart); check_int(n, >, 0); -reftable_buf_addstr (&roundtrip,"refs/heads/master" ); +check (!reftable_buf_addstr (&roundtrip,"refs/heads/master" )); m =reftable_decode_key (&roundtrip, &rt_extra, dest); check_int(n, ==, m);check (!reftable_buf_cmp (&key, &roundtrip));
@@ -469,9 +469,9 @@ static void t_reftable_index_record_comparison(void) .u.idx.last_key = REFTABLE_BUF_INIT, }, }; -reftable_buf_addstr (&in[0 ].u.idx.last_key,"refs/heads/master" ); -reftable_buf_addstr (&in[1 ].u.idx.last_key,"refs/heads/master" ); -reftable_buf_addstr (&in[2 ].u.idx.last_key,"refs/heads/branch" ); +check (!reftable_buf_addstr (&in[0 ].u.idx.last_key,"refs/heads/master" )); +check (!reftable_buf_addstr (&in[1 ].u.idx.last_key,"refs/heads/master" )); +check (!reftable_buf_addstr (&in[2 ].u.idx.last_key,"refs/heads/branch" ));check (!reftable_record_equal (&in[0 ], &in[1 ],GIT_SHA1_RAWSZ ));check (!reftable_record_cmp (&in[0 ], &in[1 ]));
@@ -510,7 +510,7 @@ static void t_reftable_index_record_roundtrip(void)int n, m;uint8_t extra; -reftable_buf_addstr (&in.u.idx.last_key,"refs/heads/master" ); +check (!reftable_buf_addstr (&in.u.idx.last_key,"refs/heads/master" ));reftable_record_key (&in, &key);t_copy (&in);
t/unit-tests/t-reftable-stack.c +29 -29
106 lines not on this page · raw message
Signature · 1 line
2.47.0.dirty
PATCH v2 7/10 reftable/stack: adapt `format_name()` to handle allocation failures
In reply to Patrick SteinhardtThe format_name() function cannot pass any errors to the caller as it has a void return type. Adapt it and its callers such that we can handle errors and start handling allocation failures.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/stack.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-)
reftable/stack.c +19 -9
@@ -623,14 +623,14 @@ int reftable_stack_add(struct reftable_stack *st,return 0 ; } -static void format_name (struct reftable_buf *dest,uint64_t min,uint64_t max) +static int format_name (struct reftable_buf *dest,uint64_t min,uint64_t max) {char buf[100 ];uint32_t rnd = (uint32_t )git_rand ();snprintf (buf,sizeof (buf),"0x%012" PRIx64"-0x%012" PRIx64"-%08x" , min, max, rnd);reftable_buf_reset (dest); -reftable_buf_addstr (dest, buf); +return reftable_buf_addstr (dest, buf); }struct reftable_addition {
@@ -846,7 +846,10 @@ int reftable_addition_add(struct reftable_addition *add,int tab_fd;reftable_buf_reset (&next_name); -format_name (&next_name, add->next_update_index, add->next_update_index); + + err =format_name (&next_name, add->next_update_index, add->next_update_index); +if (err <0 ) + goto done;stack_filename (&temp_tab_file_name, add->stack, next_name.buf);reftable_buf_addstr (&temp_tab_file_name,".temp.XXXXXX" );
@@ -893,7 +896,9 @@ int reftable_addition_add(struct reftable_addition *add, goto done; } -format_name (&next_name, wr->min_update_index, wr->max_update_index); + err =format_name (&next_name, wr->min_update_index, wr->max_update_index); +if (err <0 ) + goto done;reftable_buf_addstr (&next_name,".ref" );stack_filename (&tab_file_name, add->stack, next_name.buf);
@@ -944,9 +949,11 @@ static int stack_compact_locked(struct reftable_stack *st,struct tempfile *tab_file;int tab_fd, err =0 ; -format_name (&next_name, -reftable_reader_min_update_index (st->readers[first]), -reftable_reader_max_update_index (st->readers[last])); + err =format_name (&next_name,reftable_reader_min_update_index (st->readers[first]), +reftable_reader_max_update_index (st->readers[last])); +if (err <0 ) + goto done; +stack_filename (&tab_file_path, st, next_name.buf);reftable_buf_addstr (&tab_file_path,".temp.XXXXXX" );
@@ -1370,8 +1377,11 @@ static int stack_compact_range(struct reftable_stack *st,* it into place now. */ if (!is_empty_table) { -format_name (&new_table_name, st->readers[first]->min_update_index, - st->readers[last]->max_update_index); + err =format_name (&new_table_name, st->readers[first]->min_update_index, + st->readers[last]->max_update_index); +if (err <0 ) + goto done; +reftable_buf_addstr (&new_table_name,".ref" );stack_filename (&new_table_path, st, new_table_name.buf);
Signature · 1 line
2.47.0.dirty
PATCH v2 8/10 reftable/record: adapt `reftable_record_key()` to handle allocation failures
In reply to Patrick SteinhardtThe reftable_record_key() function cannot pass any errors to the caller as it has a void return type. Adapt it and its callers such that we can handle errors and start handling allocation failures.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/block.c | 20 +++++++++++++++----- reftable/reader.c | 8 ++++++-- reftable/record.c | 32 ++++++++++++++++++++------------ reftable/record.h | 4 ++-- reftable/writer.c | 5 ++++- 5 files changed, 47 insertions(+), 22 deletions(-)
reftable/block.c +15 -5
@@ -111,9 +111,12 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)int is_restart =0 ;struct reftable_buf key =REFTABLE_BUF_INIT ;int n =0 ; -int err =-1 ; +int err; + + err =reftable_record_key (rec, &key); +if (err <0 ) + goto done; -reftable_record_key (rec, &key);if (!key.len) { err =REFTABLE_API_ERROR ; goto done;
@@ -121,13 +124,17 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec) n =reftable_encode_key (&is_restart, out, last, key,reftable_record_val_type (rec)); -if (n <0 ) +if (n <0 ) { + err =-1 ; goto done; + }string_view_consume (&out, n); n =reftable_record_encode (rec, out, w->hash_size); -if (n <0 ) +if (n <0 ) { + err =-1 ; goto done; + }string_view_consume (&out, n); err = block_writer_register_restart(w, start.len - out.len, is_restart,
@@ -522,6 +529,10 @@ int block_iter_seek_key(struct block_iter *it, const struct block_reader *br, goto done; } + err =reftable_record_key (&rec, &it->last_key); +if (err <0 ) + goto done; +/* * Check whether the current key is greater or equal to the * sought-after key. In case it is greater we know that the
@@ -536,7 +547,6 @@ int block_iter_seek_key(struct block_iter *it, const struct block_reader *br,* to `last_key` now, and naturally all keys share a prefix * with themselves. */ -reftable_record_key (&rec, &it->last_key);if (reftable_buf_cmp (&it->last_key, want) >=0 ) { it->next_off = prev_off; goto done;
reftable/reader.c +6 -2
@@ -356,7 +356,9 @@ static int table_iter_seek_linear(struct table_iter *ti,int err;reftable_record_init (&rec,reftable_record_type (want)); -reftable_record_key (want, &want_key); + err =reftable_record_key (want, &want_key); +if (err <0 ) + goto done;/* * First we need to locate the block that must contain our record. To
@@ -439,7 +441,9 @@ static int table_iter_seek_indexed(struct table_iter *ti, };int err; -reftable_record_key (rec, &want_index.u.idx.last_key); + err =reftable_record_key (rec, &want_index.u.idx.last_key); +if (err <0 ) + goto done;/* * The index may consist of multiple levels, where each level may have
reftable/record.c +20 -12
@@ -207,12 +207,12 @@ int reftable_decode_key(struct reftable_buf *last_key, uint8_t *extra,return start_len - in.len; } -static void reftable_ref_record_key (const void *r,struct reftable_buf *dest) +static int reftable_ref_record_key (const void *r,struct reftable_buf *dest) {const struct reftable_ref_record *rec = (const struct reftable_ref_record *)r;reftable_buf_reset (dest); -reftable_buf_addstr (dest, rec->refname); +return reftable_buf_addstr (dest, rec->refname); } static int reftable_ref_record_copy_from(void *rec, const void *src_rec,
@@ -465,12 +465,12 @@ static struct reftable_record_vtable reftable_ref_record_vtable = { .cmp = &reftable_ref_record_cmp_void, }; -static void reftable_obj_record_key (const void *r,struct reftable_buf *dest) +static int reftable_obj_record_key (const void *r,struct reftable_buf *dest) {const struct reftable_obj_record *rec = (const struct reftable_obj_record *)r;reftable_buf_reset (dest); -reftable_buf_add (dest, rec->hash_prefix, rec->hash_prefix_len); +return reftable_buf_add (dest, rec->hash_prefix, rec->hash_prefix_len); }static void reftable_obj_record_release (void *rec)
@@ -664,19 +664,27 @@ static struct reftable_record_vtable reftable_obj_record_vtable = { .cmp = &reftable_obj_record_cmp_void, }; -static void reftable_log_record_key (const void *r,struct reftable_buf *dest) +static int reftable_log_record_key (const void *r,struct reftable_buf *dest) {const struct reftable_log_record *rec = (const struct reftable_log_record *)r; -int len =strlen (rec->refname); +int len =strlen (rec->refname), err;uint8_t i64[8 ];uint64_t ts =0 ; +reftable_buf_reset (dest); -reftable_buf_add (dest, (uint8_t *)rec->refname, len +1 ); + err =reftable_buf_add (dest, (uint8_t *)rec->refname, len +1 ); +if (err <0 ) +return err; ts = (~ts) - rec->update_index;put_be64 (&i64[0 ], ts); -reftable_buf_add (dest, i64,sizeof (i64)); + + err =reftable_buf_add (dest, i64,sizeof (i64)); +if (err <0 ) +return err; + +return 0 ; } static int reftable_log_record_copy_from(void *rec, const void *src_rec,
@@ -1027,11 +1035,11 @@ static struct reftable_record_vtable reftable_log_record_vtable = { .cmp = &reftable_log_record_cmp_void, }; -static void reftable_index_record_key (const void *r,struct reftable_buf *dest) +static int reftable_index_record_key (const void *r,struct reftable_buf *dest) {const struct reftable_index_record *rec = r;reftable_buf_reset (dest); -reftable_buf_add (dest, rec->last_key.buf, rec->last_key.len); +return reftable_buf_add (dest, rec->last_key.buf, rec->last_key.len); } static int reftable_index_record_copy_from(void *rec, const void *src_rec,
@@ -1124,9 +1132,9 @@ static struct reftable_record_vtable reftable_index_record_vtable = { .cmp = &reftable_index_record_cmp, }; -void reftable_record_key (struct reftable_record *rec,struct reftable_buf *dest) +int reftable_record_key (struct reftable_record *rec,struct reftable_buf *dest) { -reftable_record_vtable (rec)->key (reftable_record_data (rec), dest); +return reftable_record_vtable (rec)->key (reftable_record_data (rec), dest); } int reftable_record_encode(struct reftable_record *rec, struct string_view dest,
reftable/record.h +2 -2
@@ -40,7 +40,7 @@ int put_var_int(struct string_view *dest, uint64_t val);/* Methods for records. */ struct reftable_record_vtable {/* encode the key of to a uint8_t reftable_buf. */ -void (*key)(const void *rec,struct reftable_buf *dest); +int (*key)(const void *rec,struct reftable_buf *dest);/* The record type of ('r' for ref). */ uint8_t type;
@@ -137,7 +137,7 @@ void reftable_record_init(struct reftable_record *rec, uint8_t typ);/* see struct record_vtable */ int reftable_record_cmp (struct reftable_record *a,struct reftable_record *b);int reftable_record_equal (struct reftable_record *a,struct reftable_record *b,int hash_size); -void reftable_record_key (struct reftable_record *rec,struct reftable_buf *dest); +int reftable_record_key (struct reftable_record *rec,struct reftable_buf *dest);int reftable_record_copy_from (struct reftable_record *rec,struct reftable_record *src,int hash_size);uint8_t reftable_record_val_type (struct reftable_record *rec);
reftable/writer.c +4 -1
@@ -249,7 +249,10 @@ static int writer_add_record(struct reftable_writer *w,struct reftable_buf key =REFTABLE_BUF_INIT ;int err; -reftable_record_key (rec, &key); + err =reftable_record_key (rec, &key); +if (err <0 ) + goto done; +if (reftable_buf_cmp (&w->last_key, &key) >=0 ) { err =REFTABLE_API_ERROR ; goto done;
Signature · 1 line
2.47.0.dirty
PATCH v2 9/10 reftable/stack: adapt `stack_filename()` to handle allocation failures
In reply to Patrick SteinhardtThe stack_filename() function cannot pass any errors to the caller as it has a void return type. Adapt it and its callers such that we can handle errors and start handling allocation failures.
There are two interesting edge cases in reftable_stack_destroy() and reftable_addition_close(). Both of these are trying to tear down their respective structures, and while doing so they try to unlink some of the tables they have been keeping alive. Any earlier attempts to do that may fail on Windows because it keeps us from deleting such tables while they are still open, and thus we re-try on close. It's okay and even expected that this can fail when the tables are still open by another process, so we handle the allocation failures gracefully and just skip over any file whose name we couldn't figure out.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/stack.c | 62 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 17 deletions(-)
reftable/stack.c +45 -17
@@ -31,13 +31,16 @@ static void reftable_addition_close(struct reftable_addition *add);static int reftable_stack_reload_maybe_reuse (struct reftable_stack *st,int reuse_open); -static void stack_filename (struct reftable_buf *dest,struct reftable_stack *st, -const char *name) +static int stack_filename (struct reftable_buf *dest,struct reftable_stack *st, +const char *name) { +int err;reftable_buf_reset (dest); -reftable_buf_addstr (dest, st->reftable_dir); -reftable_buf_addstr (dest,"/" ); -reftable_buf_addstr (dest, name); +if ((err =reftable_buf_addstr (dest, st->reftable_dir)) <0 || + (err =reftable_buf_addstr (dest,"/" )) <0 || + (err =reftable_buf_addstr (dest, name)) <0 ) +return err; +return 0 ; }static ssize_t reftable_fd_write (void *arg,const void *data,size_t sz)
@@ -211,13 +214,16 @@ void reftable_stack_destroy(struct reftable_stack *st)struct reftable_buf filename =REFTABLE_BUF_INIT ;for (i =0 ; i < st->readers_len; i++) {const char *name =reader_name (st->readers[i]); +int try_unlinking =1 ; +reftable_buf_reset (&filename);if (names && !has_name (names, name)) { -stack_filename (&filename, st, name); +if (stack_filename (&filename, st, name) <0 ) + try_unlinking =0 ; }reftable_reader_decref (st->readers[i]); -if (filename.len) { +if (try_unlinking && filename.len) {/* On Windows, can only unlink after closing. */ unlink (filename.buf); }
@@ -310,7 +316,10 @@ static int reftable_stack_reload_once(struct reftable_stack *st,if (!rd) {struct reftable_block_source src = {NULL }; -stack_filename (&table_path, st, name); + + err =stack_filename (&table_path, st, name); +if (err <0 ) + goto done; err =reftable_block_source_from_file (&src, table_path.buf);
@@ -341,7 +350,11 @@ static int reftable_stack_reload_once(struct reftable_stack *st,for (i =0 ; i < cur_len; i++) {if (cur[i]) {const char *name =reader_name (cur[i]); -stack_filename (&table_path, st, name); + + err =stack_filename (&table_path, st, name); +if (err <0 ) + goto done; +reftable_reader_decref (cur[i]);unlink (table_path.buf); }
@@ -700,8 +713,8 @@ static void reftable_addition_close(struct reftable_addition *add)size_t i;for (i =0 ; i < add->new_tables_len; i++) { -stack_filename (&nm, add->stack, add->new_tables[i]); -unlink (nm.buf); +if (!stack_filename (&nm, add->stack, add->new_tables[i])) +unlink (nm.buf);reftable_free (add->new_tables[i]); add->new_tables[i] =NULL ; }
@@ -851,7 +864,9 @@ int reftable_addition_add(struct reftable_addition *add,if (err <0 ) goto done; -stack_filename (&temp_tab_file_name, add->stack, next_name.buf); + err =stack_filename (&temp_tab_file_name, add->stack, next_name.buf); +if (err <0 ) + goto done;reftable_buf_addstr (&temp_tab_file_name,".temp.XXXXXX" ); tab_file =mks_tempfile (temp_tab_file_name.buf);
@@ -900,7 +915,10 @@ int reftable_addition_add(struct reftable_addition *add,if (err <0 ) goto done;reftable_buf_addstr (&next_name,".ref" ); -stack_filename (&tab_file_name, add->stack, next_name.buf); + + err =stack_filename (&tab_file_name, add->stack, next_name.buf); +if (err <0 ) + goto done;/* On windows, this relies on rand() picking a unique destination name.
@@ -954,7 +972,9 @@ static int stack_compact_locked(struct reftable_stack *st,if (err <0 ) goto done; -stack_filename (&tab_file_path, st, next_name.buf); + err =stack_filename (&tab_file_path, st, next_name.buf); +if (err <0 ) + goto done;reftable_buf_addstr (&tab_file_path,".temp.XXXXXX" ); tab_file =mks_tempfile (tab_file_path.buf);
@@ -1174,7 +1194,9 @@ static int stack_compact_range(struct reftable_stack *st, }for (i = last +1 ; i > first; i--) { -stack_filename (&table_name, st,reader_name (st->readers[i -1 ])); + err =stack_filename (&table_name, st,reader_name (st->readers[i -1 ])); +if (err <0 ) + goto done; err =hold_lock_file_for_update (&table_locks[nlocks], table_name.buf,LOCK_NO_DEREF );
@@ -1383,7 +1405,10 @@ static int stack_compact_range(struct reftable_stack *st, goto done;reftable_buf_addstr (&new_table_name,".ref" ); -stack_filename (&new_table_path, st, new_table_name.buf); + + err =stack_filename (&new_table_path, st, new_table_name.buf); +if (err <0 ) + goto done; err =rename_tempfile (&new_table, new_table_path.buf);if (err <0 ) {
@@ -1677,7 +1702,10 @@ static void remove_maybe_stale_table(struct reftable_stack *st, uint64_t max,struct reftable_block_source src = {NULL };struct reftable_reader *rd =NULL ;struct reftable_buf table_path =REFTABLE_BUF_INIT ; -stack_filename (&table_path, st, name); + + err =stack_filename (&table_path, st, name); +if (err <0 ) + goto done; err =reftable_block_source_from_file (&src, table_path.buf);if (err <0 )
Signature · 1 line
2.47.0.dirty
PATCH v2 10/10 reftable: handle trivial `reftable_buf` errors
In reply to Patrick SteinhardtConvert the reftable library such that we handle failures with the new reftable_buf interfaces.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/block.c | 9 +++++++-- reftable/iter.c | 5 ++++- reftable/reader.c | 5 ++++- reftable/record.c | 32 +++++++++++++++++++++++-------- reftable/stack.c | 49 +++++++++++++++++++++++++++++++---------------- reftable/writer.c | 48 +++++++++++++++++++++++++++++++++------------- 6 files changed, 107 insertions(+), 41 deletions(-)
reftable/block.c +7 -2
@@ -40,7 +40,9 @@ int footer_size(int version)static int block_writer_register_restart (struct block_writer *w,int n,int is_restart,struct reftable_buf *key) { -int rlen = w->restart_len; +int rlen, err; + + rlen = w->restart_len;if (rlen >=MAX_RESTARTS ) { is_restart =0 ; }
@@ -60,7 +62,10 @@ static int block_writer_register_restart(struct block_writer *w, int n, w->next += n;reftable_buf_reset (&w->last_key); -reftable_buf_add (&w->last_key, key->buf, key->len); + err =reftable_buf_add (&w->last_key, key->buf, key->len); +if (err <0 ) +return err; + w->entries++;return 0 ; }
reftable/iter.c +4 -1
@@ -197,7 +197,10 @@ int indexed_table_ref_iter_new(struct indexed_table_ref_iter **dest, *itr = empty; itr->r = r; -reftable_buf_add (&itr->oid, oid, oid_len); + + err =reftable_buf_add (&itr->oid, oid, oid_len); +if (err <0 ) + goto out; itr->offsets = offsets; itr->offset_len = offset_len;
reftable/reader.c +4 -1
@@ -769,7 +769,10 @@ static int reftable_reader_refs_for_unindexed(struct reftable_reader *r, } *filter = empty; -reftable_buf_add (&filter->oid, oid, oid_len); + err =reftable_buf_add (&filter->oid, oid, oid_len); +if (err <0 ) + goto out; +iterator_from_table_iter (&filter->it, ti);iterator_from_filtering_ref_iterator (it, filter);
reftable/record.c +24 -8
@@ -102,7 +102,9 @@ static int decode_string(struct reftable_buf *dest, struct string_view in) {int start_len = in.len;uint64_t tsize =0 ; -int n =get_var_int (&tsize, &in); +int n, err; + + n =get_var_int (&tsize, &in);if (n <=0 )return -1 ;string_view_consume (&in, n);
@@ -110,7 +112,10 @@ static int decode_string(struct reftable_buf *dest, struct string_view in)return -1 ;reftable_buf_reset (dest); -reftable_buf_add (dest, in.buf, tsize); + err =reftable_buf_add (dest, in.buf, tsize); +if (err <0 ) +return err; +string_view_consume (&in, tsize);return start_len - in.len;
@@ -189,7 +194,7 @@ int reftable_decode_key(struct reftable_buf *last_key, uint8_t *extra,int start_len = in.len;uint64_t prefix_len =0 ;uint64_t suffix_len =0 ; -int n; +int err, n; n =reftable_decode_keylen (in, &prefix_len, &suffix_len, extra);if (n <0 )
@@ -200,8 +205,14 @@ int reftable_decode_key(struct reftable_buf *last_key, uint8_t *extra, prefix_len > last_key->len)return -1 ; -reftable_buf_setlen (last_key, prefix_len); -reftable_buf_add (last_key, in.buf, suffix_len); + err =reftable_buf_setlen (last_key, prefix_len); +if (err <0 ) +return err; + + err =reftable_buf_add (last_key, in.buf, suffix_len); +if (err <0 ) +return err; +string_view_consume (&in, suffix_len);return start_len - in.len;
@@ -1047,9 +1058,12 @@ static int reftable_index_record_copy_from(void *rec, const void *src_rec, {struct reftable_index_record *dst = rec;const struct reftable_index_record *src = src_rec; +int err;reftable_buf_reset (&dst->last_key); -reftable_buf_add (&dst->last_key, src->last_key.buf, src->last_key.len); + err =reftable_buf_add (&dst->last_key, src->last_key.buf, src->last_key.len); +if (err <0 ) +return err; dst->offset = src->offset;return 0 ;
@@ -1090,10 +1104,12 @@ static int reftable_index_record_decode(void *rec, struct reftable_buf key, {struct string_view start = in;struct reftable_index_record *r = rec; -int n =0 ; +int err, n =0 ;reftable_buf_reset (&r->last_key); -reftable_buf_add (&r->last_key, key.buf, key.len); + err =reftable_buf_add (&r->last_key, key.buf, key.len); +if (err <0 ) +return err; n =get_var_int (&r->offset, &in);if (n <0 )
reftable/stack.c +33 -16
97 lines not on this page · raw message
reftable/writer.c +35 -13
96 lines not on this page · raw message
Signature · 1 line
2.47.0.dirty
Re: [PATCH 02/10] reftable: stop using `strbuf_addf()`
In reply to karthik nayakQuoted 11 lines · karthik nayak
t/unit-tests/t-reftable-stack.c static void t_reftable_stack_auto_compaction_with_locked_tables(void) · line 1080
Quoted 5 lines · Patrick Steinhardt
t/unit-tests/t-reftable-stack.c +0
-1@@ -1077,8 +1078,10 @@ static void t_reftable_stack_auto_compaction_with_locked_tables(void)* size, we expect that auto-compaction will want to compact all of the * tables. Locking any of the tables will keep it from doing so. */ -strbuf_reset (&buf);However here it is different, since we still use the strbuf. I guess it should be okay, since 'buf' is initialized using '
STRBUF_INIT' and that still keeps the buf.len to 0.
Quoted 10 lines
t/unit-tests/t-reftable-stack.c static void t_reftable_stack_auto_compaction_with_locked_tables(void) · line 1086
Quoted 7 lines
t/unit-tests/t-reftable-stack.c +4
-1-strbuf_addf (&buf,"%s/%s.lock" , dir, st->readers[2 ]->name); +strbuf_addstr (&buf, dir); +strbuf_addstr (&buf,"/" ); +strbuf_addstr (&buf, st->readers[2 ]->name); +strbuf_addstr (&buf,".lock" );write_file_buf (buf.buf,"" ,0 );So when we do '
strbuf_addstr(&buf, ...)' it should allocate the required memory. But the reset removal did catch my eye.
Yeah, I removed it while at it as it is completely unnecessary. I've updated the commit message to point this out.
Patrick
Re: [PATCH 03/10] reftable/basics: provide new `reftable_buf` interface
In reply to karthik nayakQuoted 21 lines · karthik nayak
Quoted 20 lines · Patrick Steinhardt
reftable/basics.h +12
-0@@ -16,6 +16,22 @@ license that can be found in the LICENSE file or at#include "system.h" #include "reftable-basics.h" +struct reftable_buf { +size_t alloc; +size_t len; +char *buf; +}; +#define REFTABLE_BUF_INIT { 0 } + +void reftable_buf_init (struct reftable_buf *buf); +void reftable_buf_release (struct reftable_buf *buf); +void reftable_buf_reset (struct reftable_buf *buf); +int reftable_buf_setlen (struct reftable_buf *buf,size_t len); +int reftable_buf_cmp (const struct reftable_buf *a,const struct reftable_buf *b);
Quoted 8 lines
reftable/basics.h license that can be found in the LICENSE file or at · line 34
Quoted 4 lines
reftable/basics.h +4
-0+int reftable_buf_add (struct reftable_buf *buf,const void *data,size_t len); +int reftable_buf_addstr (struct reftable_buf *buf,const char *s); +char *reftable_buf_detach (struct reftable_buf *buf); +Nit: would be nice to have some comments explaining the functions here. I know most of them are self-explanatory and similar to strbuf, but since this is supposed to be isolated, it would be nice.
Fair enough. I was being lazy here because the code is internal, only. But that's not really a good excuse, I guess.
Patrick
Re: [PATCH 04/10] reftable: convert from `strbuf` to `reftable_buf`
In reply to karthik nayakQuoted 20 lines · karthik nayak
Quoted 5 lines · Patrick Steinhardt
Convert the reftable library to use the
reftable_bufinterface instead of thestrbufinterface. This is a mechanical change via sed(1) and does not yet handle allocation failures. These will be addressed in subsequent commits.Nit: Would be nice to list the sed command used here, so reviewers can review that instead.
[snip]
Quoted 7 lines
reftable/basics.h +0
-0@@ -104,8 +104,7 @@ char *reftable_strdup(const char *str);#endif
Quoted 5 lines
reftable/basics.h char *reftable_strdup(const char *str); · line 107
Quoted 2 lines
reftable/basics.h +0
-1/* Find the longest shared prefix size of `a` and `b` */ -struct strbuf ;I guess this is the only manual part of this commit, would be nice to mention this in the message.
Well, it's basically a lie anyway that this uses sed(1), only. Some of the functions are different between strbuf and reftable_buf, and I was too lazy to craft an sed invocation for that.
I'll adapt the message a bit to say that this is mostly using sed, but not really, without going into too many details.
Patrick
Quoted 15 lines · karthik nayak
Quoted 13 lines · Patrick Steinhardt
Hi,
this is the second patch series on my quest to make the reftable library become a standalone library again that can be used by libgit2 without pulling in all kinds of dependencies from the Git codebase. This part makes us lose the dependency on
struct strbuf, which is done due to three reasons:- To make us independent of libgit.a.
- To ensure that we use the pluggable allocators that users can set up
via `reftable_set_alloc()`.
Quoted 8 lines
Quoted 5 lines
- To make it possible to handle memory allocation failures.
While this leads to some duplication, we're only talking about ~70 lines of code.
I only have a few small comments, but overall this series looks good. Thanks
Thanks for your review!
Patrick
Re: [PATCH v2 01/10] reftable: stop using `strbuf_addbuf()`
In reply to Patrick SteinhardtQuoted 16 lines · Patrick Steinhardt
We're about to introduce our own
reftable_buftype to replacestrbuf. Get rid of the seldomly-usedstrbuf_addbuf()function such that we have to reimplement one less function.Signed-off-by Patrick Steinhardt <[email protected]>--- reftable/block.c | 2 +- reftable/record.c | 6 +++--- reftable/writer.c | 7 ++++--- 3 files changed, 8 insertions(+), 7 deletions(-)reftable/block.c +0
-0
reftable/block.c static int block_writer_register_restart(struct block_writer *w, int n, · line 66
Quoted 8 lines
reftable/block.c +1
-1@@ -60,7 +60,7 @@ static int block_writer_register_restart(struct block_writer *w, int n, w->next += n;strbuf_reset (&w->last_key); -strbuf_addbuf (&w->last_key, key); +strbuf_add (&w->last_key, key->buf, key->len); w->entries++;return 0 ; }
OK, this makes sense. FWIW, it feels like this would have been an easy function to port over to the new 'reftable_buf' type. But I understand wanting to implement fewer functions if possible.
Thanks,
Taylor
Re: [PATCH v2 02/10] reftable: stop using `strbuf_addf()`
In reply to Patrick SteinhardtQuoted 3 lines · Patrick Steinhardt
We're about to introduce our own
reftable_buftype to replacestrbuf. Get rid of the seldomly-usedstrbuf_addf()function such that we have to reimplement one less function.
Hmm. I count twelve calls to strbuf_addf() here in this patch that were rewritten in terms of snprintf()ing to a temporary buffer. So I am not sure that I agree that it is "seldomly-used".
Sure, implementing fewer functions is nice, but I am not sure that forcing the caller to use snprintf() directly is necessarily a worthwhile trade-off.
Part of me wishes that we didn't have to write our own reftable_buf in the first place. Could we use strbuf as-is and expose it through a generic reftable-specific interface that users of reftable fill in with a vtable or something?
Thanks,
Taylor
Re: [PATCH v2 03/10] reftable/basics: provide new `reftable_buf` interface
In reply to Patrick SteinhardtQuoted 8 lines · Patrick Steinhardt
Implement a new
reftable_bufinterface that will replace Git's ownstrbufinterface. This is done due to three reasons:- The
strbufinterfaces do not handle memory allocation failures and instead causes us to die. This is okay in the context of Git, but is not in the context of the reftable library, which is supposed to be usable by third-party applications.
Quoted 7 lines
- The
strbufinterface is quite deeply tied into Git, which makes it hard to use the reftable library as a standalone library. Any dependent would have to carefully extract the relevant parts of it to make things work, which is not all that sensible.- The
strbufinterface does not use the pluggable allocators that can be set up viarefatble_set_alloc().
s/refatble/reftable/.
reftable/basics.h license that can be found in the LICENSE file or at · line 63
Quoted 5 lines
reftable/basics.h +5
-0+/* +* Add the given bytes to the buffer. Returns 0 on success, +* REFTABLE_OUT_OF_MEMORY_ERROR on allocation failure. +*/ +int reftable_buf_add (struct reftable_buf *buf,const void *data,size_t len);
Is there a reason that data is a void-pointer here and not a const char *?
Thanks,
Taylor
Re: [PATCH v2 04/10] reftable: convert from `strbuf` to `reftable_buf`
In reply to Patrick SteinhardtQuoted 25 lines · Patrick Steinhardt
Convert the reftable library to use the
reftable_bufinterface instead of thestrbufinterface. This is mostly a mechanical change via sed(1) with some manual fixes where functions forstrbufandreftable_bufdiffer. The converted code does not yet handle allocation failures. This will be handled in subsequent commits.Signed-off-by Patrick Steinhardt <[email protected]>--- reftable/basics.c | 2 +- reftable/basics.h | 3 +- reftable/block.c | 34 ++++---- reftable/block.h | 14 ++-- reftable/blocksource.c | 6 +- reftable/blocksource.h | 3 +- reftable/iter.c | 6 +- reftable/iter.h | 8 +- reftable/reader.c | 16 ++-- reftable/record.c | 80 +++++++++---------- reftable/record.h | 21 ++--- reftable/stack.c | 120 ++++++++++++++-------------- reftable/system.h | 1 - reftable/writer.c | 66 +++++++-------- reftable/writer.h | 2 +- t/unit-tests/lib-reftable.c | 4 +- t/unit-tests/lib-reftable.h | 7 +-
Quoted 8 lines
t/unit-tests/t-reftable-basics.c | 16 ++-- t/unit-tests/t-reftable-block.c | 42 +++++----- t/unit-tests/t-reftable-merged.c | 26 +++--- t/unit-tests/t-reftable-reader.c | 8 +- t/unit-tests/t-reftable-readwrite.c | 92 ++++++++++----------- t/unit-tests/t-reftable-record.c | 74 ++++++++--------- t/unit-tests/t-reftable-stack.c | 90 ++++++++++----------- 24 files changed, 371 insertions(+), 370 deletions(-)
It's an awfully long patch, but was relatively easy to skim visually.
Thanks,
Taylor
Re: [PATCH v2 07/10] reftable/stack: adapt `format_name()` to handle allocation failures
In reply to Patrick Steinhardtreftable/stack.c int reftable_addition_add(struct reftable_addition *add, · line 855
Quoted 12 lines · Patrick Steinhardt
reftable/stack.c +4
-1@@ -846,7 +846,10 @@ int reftable_addition_add(struct reftable_addition *add,int tab_fd;reftable_buf_reset (&next_name); -format_name (&next_name, add->next_update_index, add->next_update_index); + + err =format_name (&next_name, add->next_update_index, add->next_update_index); +if (err <0 ) + goto done;stack_filename (&temp_tab_file_name, add->stack, next_name.buf);reftable_buf_addstr (&temp_tab_file_name,".temp.XXXXXX" );
The conversion to store the return value of 'format_name()' here makes sense. I was going to ask why this call to reftable_buf_addstr() does not have its own return value checked similarly, but I see that it is handled specially a few commits later on.
I think that what you wrote here is fine, but there are a couple of alternatives IMHO that may be worth considering in the future:
- You could do these conversions function by function, where each patch handles all potential allocation failures.
This generates more patches, but makes each individual patch a little easier to review in isolation, since the reviewer does not have to page in and out the context of what different functions do, etc.
- Alternatively, you could mention something along the lines of "this step does not make any of these functions entirely resilient against allocation failures, but future commits will address the remaining components" to avoid temporary confusion on the reader's part wondering why only part of the code appears to handle allocation failures.
Thanks,
Taylor
Quoted 6 lines · Patrick Steinhardt
Hi,
this is the second part of my patch series that stop using
struct strbufin the reftable library. This is done such that the reftable library becomes standalone again and so that we can use the pluggable allocators part of the library.
I reviewed this round, and it looks generally good to me. I feel somewhat unhappy to have to force the reftable backend to implement its own strbuf-like functionality.
So I think it may be worth considering whether or not we can reuse Git's strbuf implementation through a vtable or similar. But it may not be immediately possible since that implementation just die()s on error, can't easily swap out the allocator, etc. So perhaps this is the best path forward, it just feels somewhat unsatisfying to me.
Thanks,
Taylor
Quoted 12 lines · Taylor Blau
Quoted 6 lines · Patrick Steinhardt
Hi,
this is the second part of my patch series that stop using
struct strbufin the reftable library. This is done such that the reftable library becomes standalone again and so that we can use the pluggable allocators part of the library.I reviewed this round, and it looks generally good to me. I feel somewhat unhappy to have to force the reftable backend to implement its own strbuf-like functionality.
Quoted 5 lines
So I think it may be worth considering whether or not we can reuse Git's strbuf implementation through a vtable or similar. But it may not be immediately possible since that implementation just die()s on error, can't easily swap out the allocator, etc. So perhaps this is the best path forward, it just feels somewhat unsatisfying to me.
It's not perfect, I agree. I initially tried to do something like a vtable or to even compile this into code with something like a wrapper structure. But that approach in the end fell flat. So I decided to be pragmatic about this whole issue and just duplicate some code -- overall, we are talking about ~200 lines of code to completely detangle the reftable library from libgit.a.
For what it's worth, I also had this discussion with Junio in [1], in which we ultimately agreed that this is probably the best way forward.
Patrick
[1]: <[email protected]>
Re: [PATCH v2 02/10] reftable: stop using `strbuf_addf()`
In reply to Taylor BlauQuoted 5 lines · Taylor Blau
Quoted 3 lines · Patrick Steinhardt
We're about to introduce our own
reftable_buftype to replacestrbuf. Get rid of the seldomly-usedstrbuf_addf()function such that we have to reimplement one less function.
Quoted 7 lines
Hmm. I count twelve calls to
strbuf_addf()here in this patch that were rewritten in terms of snprintf()ing to a temporary buffer. So I am not sure that I agree that it is "seldomly-used".Sure, implementing fewer functions is nice, but I am not sure that forcing the caller to use
snprintf()directly is necessarily a worthwhile trade-off.
Another problem here is that snprintf() isn't exactly the most portable interface. Some systems don't have it at all, others have broken return code handling. So avoiding it completely makes that issue go away entirely.
I can add that to the commit message if this needs a reroll.
Quoted 4 lines
Part of me wishes that we didn't have to write our own
reftable_bufin the first place. Could we usestrbufas-is and expose it through a generic reftable-specific interface that users of reftable fill in with a vtable or something?
I tried that, and it felt way worse. The amount of code you have to write is roughly in the same ballpark, you don't have pluggable allocators, you don't have allocation error handling and every consumer would have to implement their own type.
So overall it's only losses from my point of view.
Patrick
Re: [PATCH v2 03/10] reftable/basics: provide new `reftable_buf` interface
In reply to Taylor BlauQuoted 19 lines · Taylor Blau
Quoted 8 lines · Patrick Steinhardt
Implement a new
reftable_bufinterface that will replace Git's ownstrbufinterface. This is done due to three reasons:- The
strbufinterfaces do not handle memory allocation failures and instead causes us to die. This is okay in the context of Git, but is not in the context of the reftable library, which is supposed to be usable by third-party applications.Quoted 7 lines
- The
strbufinterface is quite deeply tied into Git, which makes it hard to use the reftable library as a standalone library. Any dependent would have to carefully extract the relevant parts of it to make things work, which is not all that sensible.- The
strbufinterface does not use the pluggable allocators that can be set up viarefatble_set_alloc().s/refatble/reftable/.
Quoted 8 lines
reftable/basics.h license that can be found in the LICENSE file or at · line 63
Quoted 5 lines
reftable/basics.h +5
-0+/* +* Add the given bytes to the buffer. Returns 0 on success, +* REFTABLE_OUT_OF_MEMORY_ERROR on allocation failure. +*/ +int reftable_buf_add (struct reftable_buf *buf,const void *data,size_t len);Is there a reason that data is a void-pointer here and not a const char *?
Only that it emulates strbuf_add(), which also uses a void pointer.
Patrick
Re: [PATCH v2 03/10] reftable/basics: provide new `reftable_buf` interface
Quoted 11 lines · Patrick Steinhardt
Quoted 9 lines · Taylor Blau
reftable/basics.h license that can be found in the LICENSE file or at · line 63
Quoted 5 lines · Patrick Steinhardt
reftable/basics.h +5
-0+/* +* Add the given bytes to the buffer. Returns 0 on success, +* REFTABLE_OUT_OF_MEMORY_ERROR on allocation failure. +*/ +int reftable_buf_add (struct reftable_buf *buf,const void *data,size_t len);Is there a reason that data is a void-pointer here and not a const char *?
Quoted 1 line
Only that it emulates
strbuf_add(), which also uses a void pointer.
The reason for that is because strbuf is a generic byte-array which may contain embedded NULs, and the const void * plus len emphasizes this property, whereas const char * would imply a C-string with no embedded NULs.
(strbuf also happens to ensure that the contained byte-array ends with NUL so that the .buf member can be used safely with any function accepting a C-string, but that is a convenience for the common case when it does not contain embedded NULs, not a promise that there won't be embedded NULs, hence const void * rather than cont char *.)
Quoted 19 lines · Patrick Steinhardt
Quoted 12 lines · Taylor Blau
Quoted 6 lines · Patrick Steinhardt
Hi,
this is the second part of my patch series that stop using
struct strbufin the reftable library. This is done such that the reftable library becomes standalone again and so that we can use the pluggable allocators part of the library.I reviewed this round, and it looks generally good to me. I feel somewhat unhappy to have to force the reftable backend to implement its own strbuf-like functionality.
Quoted 5 lines
So I think it may be worth considering whether or not we can reuse Git's strbuf implementation through a vtable or similar. But it may not be immediately possible since that implementation just die()s on error, can't easily swap out the allocator, etc. So perhaps this is the best path forward, it just feels somewhat unsatisfying to me.
Quoted 7 lines
It's not perfect, I agree. I initially tried to do something like a vtable or to even compile this into code with something like a wrapper structure. But that approach in the end fell flat. So I decided to be pragmatic about this whole issue and just duplicate some code -- overall, we are talking about ~200 lines of code to completely detangle the reftable library from libgit.a.
I have read some patches yesterday, I feel quite strange that we need to make repetition. Could we provide a header file which requires the users who need to use the reftable library to implement the interfaces?
reftable_strbuf_addf(void *buf, char *fmt, va_list ap);
Thus, we could reuse "strbuf_addf" to implement this interface in Git. As for libgit2, could we let it implement these interfaces? Although I have never read the source code of libgit2, I think there should be some code which could be reuse to implement these interfaces?
However, I do not know the context. Maybe the above is totally wrong. If so, please ignore.
Thanks,
Jialuo
Quoted 34 lines · shejialuo
Quoted 19 lines · Patrick Steinhardt
Quoted 12 lines · Taylor Blau
Quoted 6 lines · Patrick Steinhardt
Hi,
this is the second part of my patch series that stop using
struct strbufin the reftable library. This is done such that the reftable library becomes standalone again and so that we can use the pluggable allocators part of the library.I reviewed this round, and it looks generally good to me. I feel somewhat unhappy to have to force the reftable backend to implement its own strbuf-like functionality.
Quoted 5 lines
So I think it may be worth considering whether or not we can reuse Git's strbuf implementation through a vtable or similar. But it may not be immediately possible since that implementation just die()s on error, can't easily swap out the allocator, etc. So perhaps this is the best path forward, it just feels somewhat unsatisfying to me.
Quoted 7 lines
It's not perfect, I agree. I initially tried to do something like a vtable or to even compile this into code with something like a wrapper structure. But that approach in the end fell flat. So I decided to be pragmatic about this whole issue and just duplicate some code -- overall, we are talking about ~200 lines of code to completely detangle the reftable library from libgit.a.
I have read some patches yesterday, I feel quite strange that we need to make repetition. Could we provide a header file which requires the users who need to use the reftable library to implement the interfaces?
reftable_strbuf_addf(void *buf, char *fmt, va_list ap);
Quoted 7 lines
Thus, we could reuse "
strbuf_addf" to implement this interface in Git. As for libgit2, could we let it implement these interfaces? Although I have never read the source code of libgit2, I think there should be some code which could be reuse to implement these interfaces?However, I do not know the context. Maybe the above is totally wrong. If so, please ignore.
The thing is that we'll have repetition regardless of what we end up doing:
- We could either have repetition once in the reftable library, reimplementing struct strbuf. This can then be reused by every single user of the reftable library.
- Or we can have repetition for every single user of the reftable library. For now that'd only be Git and libgit2, but we'd still have repetition.
The second kind of repetition is way worse though, because now every user of the reftable library has a different implementation of a type that is as basic as a buffer. These _must_ behave the exact same across implementations or we will hit issues. So I'd rather have the repetition a single time in the reftable library such that all users of the library will behave the same rather than having downstream users copy the implementation of struct strbuf and making it work for their library.
Also, due to the nature of struct strbuf not allowing for allocation failures we'd already have diverging behaviour. In Git you would never hit error code paths for allocation failures, whereas every library user potentially can.
So we really have to treat the reftable code base special. If we want to be a good citizen and be a proper upstream for projects like libgit2 we don't really have much of a choice than to detangle it from libgit.a. If we don't we may be saving 20 lines of code, but we make everybody elses life harder.
Patrick
On Tue, Oct 15, 2024 at 12:44:53PM +0200, Patrick Steinhardt wrote:
[snip]
Quoted 25 lines
Quoted 6 lines
I have read some patches yesterday, I feel quite strange that we need to make repetition. Could we provide a header file which requires the users who need to use the reftable library to implement the interfaces?
reftable_strbuf_addf(void *buf, char *fmt, va_list ap);Quoted 7 lines
Thus, we could reuse "
strbuf_addf" to implement this interface in Git. As for libgit2, could we let it implement these interfaces? Although I have never read the source code of libgit2, I think there should be some code which could be reuse to implement these interfaces?However, I do not know the context. Maybe the above is totally wrong. If so, please ignore.
The thing is that we'll have repetition regardless of what we end up doing:
- We could either have repetition once in the reftable library, reimplementing
struct strbuf. This can then be reused by every single user of the reftable library.- Or we can have repetition for every single user of the reftable library. For now that'd only be Git and libgit2, but we'd still have repetition.
Quoted 8 lines
The second kind of repetition is way worse though, because now every user of the reftable library has a different implementation of a type that is as basic as a buffer. These _must_ behave the exact same across implementations or we will hit issues. So I'd rather have the repetition a single time in the reftable library such that all users of the library will behave the same rather than having downstream users copy the implementation of
struct strbufand making it work for their library.
Yes. I agree with you it is worse to let every downstream to implement the interfaces. I know the motivation here, we want to make the whole reftable library be independent of the Git which allows the downstream to easily use the reftable library.
Quoted 11 lines
Also, due to the nature of
struct strbufnot allowing for allocation failures we'd already have diverging behaviour. In Git you would never hit error code paths for allocation failures, whereas every library user potentially can.So we really have to treat the reftable code base special. If we want to be a good citizen and be a proper upstream for projects like libgit2 we don't really have much of a choice than to detangle it from libgit.a. If we don't we may be saving 20 lines of code, but we make everybody elses life harder.
Yes. And I do not think this is a problem right now. Thanks for this wonderful explanation.
Quoted 1 line
Patrick
Jialuo
Re: [PATCH v2 02/10] reftable: stop using `strbuf_addf()`
In reply to Patrick SteinhardtQuoted 11 lines · Patrick Steinhardt
Quoted 4 lines
Part of me wishes that we didn't have to write our own
reftable_bufin the first place. Could we usestrbufas-is and expose it through a generic reftable-specific interface that users of reftable fill in with a vtable or something?I tried that, and it felt way worse. The amount of code you have to write is roughly in the same ballpark, you don't have pluggable allocators, you don't have allocation error handling and every consumer would have to implement their own type.
So overall it's only losses from my point of view.
Makes sense, although the end result is somewhat unsatisfying.
Thanks,
Taylor
Re: [PATCH v2 03/10] reftable/basics: provide new `reftable_buf` interface
In reply to Eric SunshineQuoted 12 lines · Eric Sunshine
Quoted 11 lines · Patrick Steinhardt
Quoted 9 lines · Taylor Blau
reftable/basics.h license that can be found in the LICENSE file or at · line 63
Quoted 5 lines · Patrick Steinhardt
reftable/basics.h +5
-0+/* +* Add the given bytes to the buffer. Returns 0 on success, +* REFTABLE_OUT_OF_MEMORY_ERROR on allocation failure. +*/ +int reftable_buf_add (struct reftable_buf *buf,const void *data,size_t len);Is there a reason that data is a void-pointer here and not a const char *?
Quoted 6 lines
Quoted 1 line
Only that it emulates
strbuf_add(), which also uses a void pointer.The reason for that is because strbuf is a generic byte-array which may contain embedded NULs, and the
const void *pluslenemphasizes this property, whereasconst char *would imply a C-string with no embedded NULs.
Thanks, that was the explanation I was missing. Perhaps it is worth re-stating in the commit message here to avoid confusing readers like I was when I first read Patrick's patch ;-).
Thanks,
Taylor
Re: [PATCH v2 03/10] reftable/basics: provide new `reftable_buf` interface
Quoted 15 lines · Taylor Blau
Quoted 14 lines · Eric Sunshine
Quoted 12 lines · Patrick Steinhardt
Quoted 9 lines · Taylor Blau
reftable/basics.h license that can be found in the LICENSE file or at · line 63
Quoted 5 lines · Patrick Steinhardt
reftable/basics.h +5
-0+/* +* Add the given bytes to the buffer. Returns 0 on success, +* REFTABLE_OUT_OF_MEMORY_ERROR on allocation failure. +*/ +int reftable_buf_add (struct reftable_buf *buf,const void *data,size_t len);Is there a reason that data is a void-pointer here and not a const char *?
Only that it emulates
strbuf_add(), which also uses a void pointer.
Quoted 8 lines
Quoted 4 lines
The reason for that is because strbuf is a generic byte-array which may contain embedded NULs, and the
const void *pluslenemphasizes this property, whereasconst char *would imply a C-string with no embedded NULs.Thanks, that was the explanation I was missing. Perhaps it is worth re-stating in the commit message here to avoid confusing readers like I was when I first read Patrick's patch ;-).
Does it make sense to explicitly state how the interfaces look like though? I don't do that for the other functions either, and for most of the part I just reuse the exact same function arguments as we had with the strbuf interface.
Patrick
Re: [PATCH v2 03/10] reftable/basics: provide new `reftable_buf` interface
Quoted 21 lines · Patrick Steinhardt
Quoted 20 lines · Taylor Blau
Quoted 18 lines · Eric Sunshine
Quoted 11 lines · Patrick Steinhardt
Quoted 9 lines · Taylor Blau
reftable/basics.h license that can be found in the LICENSE file or at · line 63
Quoted 5 lines · Patrick Steinhardt
reftable/basics.h +5
-0+/* +* Add the given bytes to the buffer. Returns 0 on success, +* REFTABLE_OUT_OF_MEMORY_ERROR on allocation failure. +*/ +int reftable_buf_add (struct reftable_buf *buf,const void *data,size_t len);Is there a reason that data is a void-pointer here and not a const char *?
Quoted 1 line
Only that it emulates
strbuf_add(), which also uses a void pointer.The reason for that is because strbuf is a generic byte-array which may contain embedded NULs, and the
const void *pluslenemphasizes this property, whereasconst char *would imply a C-string with no embedded NULs.
Quoted 8 lines
Quoted 3 lines
Thanks, that was the explanation I was missing. Perhaps it is worth re-stating in the commit message here to avoid confusing readers like I was when I first read Patrick's patch ;-).
Does it make sense to explicitly state how the interfaces look like though? I don't do that for the other functions either, and for most of the part I just reuse the exact same function arguments as we had with the strbuf interface.
I don't feel very strongly about it, but I had suggested it because my initial read of this patch confused me, and I had wondered if others may be similarly confused.
For what it's worth, I was thinking something on the order of the following added to the patch message:
Note that the reftable_buf_add() function intentionally takes a "const void *" instead of a "const char *" (as does its strbuf counterpart, strbuf_add()) to emphasize that the buffer may contain NUL characters.
But, as I said, I don't feel very strongly about it.
Thanks,
Taylor
v3 cover reftable: stop using `struct strbuf`
In reply to Patrick SteinhardtHi,
this is the third version of my patch series that removes use of struct strbuf in the reftable library. The intent of this is to convert the reftable library back into a standalone library that can be used in the context of libgit2.
Changes compared to v2:
- Provide more context around why we get rid of stbuf_addf().
- Fix a commit message type.
- Provide better docs for reftable_buf_add().
Thanks!
Patrick
Patrick Steinhardt (10):
reftable: stop using strbuf_addbuf()
reftable: stop using strbuf_addf()
reftable/basics: provide new reftable_buf interface
reftable: convert from strbuf to reftable_buf reftable/blocksource: adapt interface name
t/unit-tests: check for reftable_buf allocation errors reftable/stack: adapt format_name() to handle allocation failures reftable/record: adapt reftable_record_key() to handle allocation
failures
reftable/stack: adapt stack_filename() to handle allocation failures
reftable: handle trivial reftable_buf errors
reftable/basics.c | 76 +++++++++- reftable/basics.h | 61 +++++++- reftable/block.c | 61 +++++--- reftable/block.h | 14 +- reftable/blocksource.c | 30 ++-- reftable/blocksource.h | 5 +- reftable/iter.c | 9 +- reftable/iter.h | 8 +- reftable/reader.c | 27 ++-- reftable/record.c | 114 ++++++++------ reftable/record.h | 21 +-- reftable/stack.c | 221 ++++++++++++++++++---------- reftable/system.h | 1 - reftable/writer.c | 102 ++++++++----- reftable/writer.h | 2 +- t/unit-tests/lib-reftable.c | 4 +- t/unit-tests/lib-reftable.h | 7 +- t/unit-tests/t-reftable-basics.c | 16 +- t/unit-tests/t-reftable-block.c | 53 +++---- t/unit-tests/t-reftable-merged.c | 32 ++-- t/unit-tests/t-reftable-reader.c | 12 +- t/unit-tests/t-reftable-readwrite.c | 134 +++++++++-------- t/unit-tests/t-reftable-record.c | 74 +++++----- t/unit-tests/t-reftable-stack.c | 96 ++++++------ 24 files changed, 728 insertions(+), 452 deletions(-)
Range-diff against v2:
1: 7408482c152 = 1: 7408482c152 reftable: stop using `strbuf_addbuf()` 2: 6a7333b275e ! 2: 634fd3c35f5 reftable: stop using `strbuf_addf()`
@@ Commit message
reftable: stop using strbuf_addf()
We're about to introduce our own reftable_buf type to replace
- strbuf. Get rid of the seldomly-used strbuf_addf() function such
- that we have to reimplement one less function.
+ strbuf. One function we'll have to convert is strbuf_addf(), which
+ is used in a handful of places. This function uses snprintf()
+ internally, which makes porting it a bit more involved:
+
+ - It is not available on all platforms.
+
+ - Some platforms like Windows have broken implementations.
+
+ So by using snprintf() we'd also push the burden on downstream users
+ of the reftable library to make available a properly working version of
+ it.
+
+ Most callsites of strbuf_addf() are trivial to convert to not using
+ it. We do end up using snprintf() in our unit tests, but that isn't
+ much of a problem for downstream users of the reftable library.
While at it, remove a useless call to strbuf_reset() in t_reftable_stack_auto_compaction_with_locked_tables(). We don't write
3: 0ddc8c0c896 ! 3: 53c5f667f28 reftable/basics: provide new reftable_buf interface
@@ Commit message
to make things work, which is not all that sensible.
- The strbuf interface does not use the pluggable allocators that
- can be set up via refatble_set_alloc().
+ can be set up via reftable_set_alloc().
So we have good reasons to use our own type, and the implementation is rather trivial. Implement our own type. Conversion of the reftable
4: e1ff1af1f30 ! 4: 7c7ccc5d966 reftable: convert from strbuf to reftable_buf
@@ reftable/basics.c: int names_equal(const char **a, const char **b)
for (; p < a->len && p < b->len; p++) {
## reftable/basics.h ##
+@@ reftable/basics.h: int reftable_buf_setlen(struct reftable_buf *buf, size_t len);
+ int reftable_buf_cmp(const struct reftable_buf *a, const struct reftable_buf *b); +
+ /*
+- * Add the given bytes to the buffer. Returns 0 on success,
++ * Append len bytes from data to the buffer. This function works with ++ * arbitrary byte sequences, including ones that contain embedded NUL ++ * characters. As such, we use void * as input type. Returns 0 on success,
+ * REFTABLE_OUT_OF_MEMORY_ERROR on allocation failure.
+ */
+ int reftable_buf_add(struct reftable_buf *buf, const void *data, size_t len); @@ reftable/basics.h: char *reftable_strdup(const char *str);
#endif
5: fe8c9ace463 = 5: f9632860933 reftable/blocksource: adapt interface name 6: 8c98745233a = 6: d850a2fe7d0 t/unit-tests: check for `reftable_buf` allocation errors 7: 1f08163009b = 7: 8f8e2ca3962 reftable/stack: adapt `format_name()` to handle allocation failures 8: 5798d76d7a4 = 8: 268e4cd6fc6 reftable/record: adapt `reftable_record_key()` to handle allocation failures 9: a9582d51dd1 = 9: 245a428842a reftable/stack: adapt `stack_filename()` to handle allocation failures 10: 90819c90f38 = 10: 4b51ea4b628 reftable: handle trivial `reftable_buf` errors
Signature · 1 line
2.47.0.72.gef8ce8f3d4.dirty
PATCH v3 1/10 reftable: stop using `strbuf_addbuf()`
We're about to introduce our own reftable_buf type to replace strbuf. Get rid of the seldomly-used strbuf_addbuf() function such that we have to reimplement one less function.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/block.c | 2 +- reftable/record.c | 6 +++--- reftable/writer.c | 7 ++++--- 3 files changed, 8 insertions(+), 7 deletions(-)
reftable/block.c +1 -1
@@ -60,7 +60,7 @@ static int block_writer_register_restart(struct block_writer *w, int n, w->next += n;strbuf_reset (&w->last_key); -strbuf_addbuf (&w->last_key, key); +strbuf_add (&w->last_key, key->buf, key->len); w->entries++;return 0 ; }
reftable/record.c +3 -3
@@ -1031,7 +1031,7 @@ static void reftable_index_record_key(const void *r, struct strbuf *dest) {const struct reftable_index_record *rec = r;strbuf_reset (dest); -strbuf_addbuf (dest, &rec->last_key); +strbuf_add (dest, rec->last_key.buf, rec->last_key.len); } static int reftable_index_record_copy_from(void *rec, const void *src_rec,
@@ -1041,7 +1041,7 @@ static int reftable_index_record_copy_from(void *rec, const void *src_rec,const struct reftable_index_record *src = src_rec;strbuf_reset (&dst->last_key); -strbuf_addbuf (&dst->last_key, &src->last_key); +strbuf_add (&dst->last_key, src->last_key.buf, src->last_key.len); dst->offset = src->offset;return 0 ;
@@ -1085,7 +1085,7 @@ static int reftable_index_record_decode(void *rec, struct strbuf key,int n =0 ;strbuf_reset (&r->last_key); -strbuf_addbuf (&r->last_key, &key); +strbuf_add (&r->last_key, key.buf, key.len); n =get_var_int (&r->offset, &in);if (n <0 )
reftable/writer.c +4 -3
@@ -225,7 +225,7 @@ static int writer_index_hash(struct reftable_writer *w, struct strbuf *hash) *key = empty;strbuf_reset (&key->hash); -strbuf_addbuf (&key->hash, hash); +strbuf_add (&key->hash, hash->buf, hash->len);tree_insert (&w->obj_index_tree, key, &obj_index_tree_node_compare); }else {
@@ -256,7 +256,7 @@ static int writer_add_record(struct reftable_writer *w, }strbuf_reset (&w->last_key); -strbuf_addbuf (&w->last_key, &key); +strbuf_add (&w->last_key, key.buf, key.len);if (!w->block_writer) { err =writer_reinit_block_writer (w,reftable_record_type (rec));if (err <0 )
@@ -778,7 +778,8 @@ static int writer_flush_nonempty_block(struct reftable_writer *w) index_record.offset = w->next;strbuf_reset (&index_record.last_key); -strbuf_addbuf (&index_record.last_key, &w->block_writer->last_key); +strbuf_add (&index_record.last_key, w->block_writer->last_key.buf, + w->block_writer->last_key.len); w->index[w->index_len] = index_record; w->index_len++;
Signature · 1 line
2.47.0.72.gef8ce8f3d4.dirty
PATCH v3 2/10 reftable: stop using `strbuf_addf()`
In reply to Patrick SteinhardtWe're about to introduce our own reftable_buf type to replace strbuf. One function we'll have to convert is strbuf_addf(), which is used in a handful of places. This function uses snprintf() internally, which makes porting it a bit more involved:
- It is not available on all platforms.
- Some platforms like Windows have broken implementations.
So by using snprintf() we'd also push the burden on downstream users of the reftable library to make available a properly working version of it.
Most callsites of strbuf_addf() are trivial to convert to not using it. We do end up using snprintf() in our unit tests, but that isn't much of a problem for downstream users of the reftable library.
While at it, remove a useless call to strbuf_reset() in t_reftable_stack_auto_compaction_with_locked_tables(). We don't write to the buffer before this and initialize it with STRBUF_INIT, so there is no need to reset anything.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/stack.c | 18 ++++++++----- t/unit-tests/t-reftable-block.c | 7 +++-- t/unit-tests/t-reftable-readwrite.c | 20 +++++++------- t/unit-tests/t-reftable-stack.c | 42 ++++++++++++++++------------- 4 files changed, 50 insertions(+), 37 deletions(-)
reftable/stack.c +12 -6
@@ -1387,12 +1387,18 @@ static int stack_compact_range(struct reftable_stack *st,* have just written. In case the compacted table became empty we * simply skip writing it. */ -for (i =0 ; i < first_to_replace; i++) -strbuf_addf (&tables_list_buf,"%s\n" , names[i]); -if (!is_empty_table) -strbuf_addf (&tables_list_buf,"%s\n" , new_table_name.buf); -for (i = last_to_replace +1 ; names[i]; i++) -strbuf_addf (&tables_list_buf,"%s\n" , names[i]); +for (i =0 ; i < first_to_replace; i++) { +strbuf_addstr (&tables_list_buf, names[i]); +strbuf_addstr (&tables_list_buf,"\n" ); + } +if (!is_empty_table) { +strbuf_addstr (&tables_list_buf, new_table_name.buf); +strbuf_addstr (&tables_list_buf,"\n" ); + } +for (i = last_to_replace +1 ; names[i]; i++) { +strbuf_addstr (&tables_list_buf, names[i]); +strbuf_addstr (&tables_list_buf,"\n" ); + } err =write_in_full (get_lock_file_fd (&tables_list_lock), tables_list_buf.buf, tables_list_buf.len);
t/unit-tests/t-reftable-block.c +5 -2
@@ -308,10 +308,13 @@ static void t_index_block_read_write(void)check (!ret);for (i =0 ; i <N ; i++) { -strbuf_init (&recs[i].u.idx.last_key,9 ); +char buf[128 ]; + +snprintf (buf,sizeof (buf),"branch%02" PRIuMAX, (uintmax_t )i); +strbuf_init (&recs[i].u.idx.last_key,9 ); recs[i].type =BLOCK_TYPE_INDEX ; -strbuf_addf (&recs[i].u.idx.last_key,"branch%02" PRIuMAX, (uintmax_t )i); +strbuf_addstr (&recs[i].u.idx.last_key, buf); recs[i].u.idx.offset = i; ret =block_writer_add (&bw, &recs[i]);
t/unit-tests/t-reftable-readwrite.c +9 -11
@@ -753,12 +753,13 @@ static void t_write_multiple_indices(void)struct reftable_write_options opts = { .block_size =100 , }; -struct strbuf writer_buf =STRBUF_INIT , buf =STRBUF_INIT ; +struct strbuf writer_buf =STRBUF_INIT ;struct reftable_block_source source = {0 };struct reftable_iterator it = {0 };const struct reftable_stats *stats;struct reftable_writer *writer;struct reftable_reader *reader; +char buf[128 ];int err, i; writer =t_reftable_strbuf_writer (&writer_buf, &opts);
@@ -770,9 +771,8 @@ static void t_write_multiple_indices(void) .value.val1 = {i}, }; -strbuf_reset (&buf); -strbuf_addf (&buf,"refs/heads/%04d" , i); - ref.refname = buf.buf, +snprintf (buf,sizeof (buf),"refs/heads/%04d" , i); + ref.refname = buf; err =reftable_writer_add_ref (writer, &ref);check (!err);
@@ -788,9 +788,8 @@ static void t_write_multiple_indices(void) }, }; -strbuf_reset (&buf); -strbuf_addf (&buf,"refs/heads/%04d" , i); - log.refname = buf.buf, +snprintf (buf,sizeof (buf),"refs/heads/%04d" , i); + log.refname = buf; err =reftable_writer_add_log (writer, &log);check (!err);
@@ -824,7 +823,6 @@ static void t_write_multiple_indices(void)reftable_writer_free (writer);reftable_reader_decref (reader);strbuf_release (&writer_buf); -strbuf_release (&buf); }static void t_write_multi_level_index (void )
@@ -848,10 +846,10 @@ static void t_write_multi_level_index(void) .value_type = REFTABLE_REF_VAL1, .value.val1 = {i}, }; +char buf[128 ]; -strbuf_reset (&buf); -strbuf_addf (&buf,"refs/heads/%03" PRIuMAX, (uintmax_t )i); - ref.refname = buf.buf, +snprintf (buf,sizeof (buf),"refs/heads/%03" PRIuMAX, (uintmax_t )i); + ref.refname = buf; err =reftable_writer_add_ref (writer, &ref);check (!err);
t/unit-tests/t-reftable-stack.c +24 -18
110 lines not on this page · raw message
Signature · 1 line
2.47.0.72.gef8ce8f3d4.dirty
PATCH v3 3/10 reftable/basics: provide new `reftable_buf` interface
In reply to Patrick SteinhardtImplement a new reftable_buf interface that will replace Git's own strbuf interface. This is done due to three reasons:
- The strbuf interfaces do not handle memory allocation failures and instead causes us to die. This is okay in the context of Git, but is not in the context of the reftable library, which is supposed to be usable by third-party applications.
- The strbuf interface is quite deeply tied into Git, which makes it hard to use the reftable library as a standalone library. Any dependent would have to carefully extract the relevant parts of it to make things work, which is not all that sensible.
- The strbuf interface does not use the pluggable allocators that can be set up via reftable_set_alloc().
So we have good reasons to use our own type, and the implementation is rather trivial. Implement our own type. Conversion of the reftable library will be handled in subsequent commits.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/basics.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++ reftable/basics.h | 56 +++++++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+)
reftable/basics.c +74 -0
@@ -9,6 +9,7 @@ license that can be found in the LICENSE file or at#define REFTABLE_ALLOW_BANNED_ALLOCATORS #include "basics.h" #include "reftable-basics.h" +#include "reftable-error.h" static void *(*reftable_malloc_ptr)(size_t sz);static void *(*reftable_realloc_ptr)(void *,size_t );
@@ -69,6 +70,79 @@ void reftable_set_alloc(void *(*malloc)(size_t), reftable_free_ptr = free; } +void reftable_buf_init (struct reftable_buf *buf) +{ +struct reftable_buf empty =REFTABLE_BUF_INIT ; + *buf = empty; +} + +void reftable_buf_release (struct reftable_buf *buf) +{ +reftable_free (buf->buf); +reftable_buf_init (buf); +} + +void reftable_buf_reset (struct reftable_buf *buf) +{ +if (buf->alloc) { + buf->len =0 ; + buf->buf[0 ] ='\0' ; + } +} + +int reftable_buf_setlen (struct reftable_buf *buf,size_t len) +{ +if (len > buf->len) +return -1 ; +if (len == buf->len) +return 0 ; + buf->buf[len] ='\0' ; + buf->len = len; +return 0 ; +} + +int reftable_buf_cmp (const struct reftable_buf *a,const struct reftable_buf *b) +{ +size_t len = a->len < b->len ? a->len : b->len; +if (len) { +int cmp =memcmp (a->buf, b->buf, len); +if (cmp) +return cmp; + } +return a->len < b->len ?-1 : a->len != b->len; +} + +int reftable_buf_add (struct reftable_buf *buf,const void *data,size_t len) +{ +size_t newlen = buf->len + len; + +if (newlen +1 > buf->alloc) { +char *reallocated = buf->buf; +REFTABLE_ALLOC_GROW (reallocated, newlen +1 , buf->alloc); +if (!reallocated) +return REFTABLE_OUT_OF_MEMORY_ERROR ; + buf->buf = reallocated; + } + +memcpy (buf->buf + buf->len, data, len); + buf->buf[newlen] ='\0' ; + buf->len = newlen; + +return 0 ; +} + +int reftable_buf_addstr (struct reftable_buf *buf,const char *s) +{ +return reftable_buf_add (buf, s,strlen (s)); +} + +char *reftable_buf_detach (struct reftable_buf *buf) +{ +char *result = buf->buf; +reftable_buf_init (buf); +return result; +} +void put_be24 (uint8_t *out,uint32_t i) { out[0 ] = (uint8_t )((i >>16 ) &0xff );
reftable/basics.h +56 -0
62 lines not on this page · raw message
Signature · 1 line
2.47.0.72.gef8ce8f3d4.dirty
PATCH v3 4/10 reftable: convert from `strbuf` to `reftable_buf`
In reply to Patrick SteinhardtConvert the reftable library to use the reftable_buf interface instead of the strbuf interface. This is mostly a mechanical change via sed(1) with some manual fixes where functions for strbuf and reftable_buf differ. The converted code does not yet handle allocation failures. This will be handled in subsequent commits.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/basics.c | 2 +- reftable/basics.h | 7 +- reftable/block.c | 34 ++++---- reftable/block.h | 14 ++-- reftable/blocksource.c | 6 +- reftable/blocksource.h | 3 +- reftable/iter.c | 6 +- reftable/iter.h | 8 +- reftable/reader.c | 16 ++-- reftable/record.c | 80 +++++++++---------- reftable/record.h | 21 ++--- reftable/stack.c | 120 ++++++++++++++-------------- reftable/system.h | 1 - reftable/writer.c | 66 +++++++-------- reftable/writer.h | 2 +- t/unit-tests/lib-reftable.c | 4 +- t/unit-tests/lib-reftable.h | 7 +- t/unit-tests/t-reftable-basics.c | 16 ++-- t/unit-tests/t-reftable-block.c | 42 +++++----- t/unit-tests/t-reftable-merged.c | 26 +++--- t/unit-tests/t-reftable-reader.c | 8 +- t/unit-tests/t-reftable-readwrite.c | 92 ++++++++++----------- t/unit-tests/t-reftable-record.c | 74 ++++++++--------- t/unit-tests/t-reftable-stack.c | 90 ++++++++++----------- 24 files changed, 374 insertions(+), 371 deletions(-)
reftable/basics.c +1 -1
@@ -260,7 +260,7 @@ int names_equal(const char **a, const char **b)return a[i] == b[i]; } -int common_prefix_size (struct strbuf *a,struct strbuf *b) +int common_prefix_size (struct reftable_buf *a,struct reftable_buf *b) {int p =0 ;for (; p < a->len && p < b->len; p++) {
reftable/basics.h +4 -3
@@ -57,7 +57,9 @@ int reftable_buf_setlen(struct reftable_buf *buf, size_t len);int reftable_buf_cmp (const struct reftable_buf *a,const struct reftable_buf *b);/* -* Add the given bytes to the buffer. Returns 0 on success, +* Append `len` bytes from `data` to the buffer. This function works with +* arbitrary byte sequences, including ones that contain embedded NUL +* characters. As such, we use `void *` as input type. Returns 0 on success, * REFTABLE_OUT_OF_MEMORY_ERROR on allocation failure. */ int reftable_buf_add (struct reftable_buf *buf,const void *data,size_t len);
@@ -144,8 +146,7 @@ char *reftable_strdup(const char *str);#endif /* Find the longest shared prefix size of `a` and `b` */ -struct strbuf ; -int common_prefix_size (struct strbuf *a,struct strbuf *b); +int common_prefix_size (struct reftable_buf *a,struct reftable_buf *b);int hash_size (uint32_t id);
reftable/block.c +17 -17
@@ -38,7 +38,7 @@ int footer_size(int version) }static int block_writer_register_restart (struct block_writer *w,int n, -int is_restart,struct strbuf *key) +int is_restart,struct reftable_buf *key) {int rlen = w->restart_len;if (rlen >=MAX_RESTARTS ) {
@@ -59,8 +59,8 @@ static int block_writer_register_restart(struct block_writer *w, int n, w->next += n; -strbuf_reset (&w->last_key); -strbuf_add (&w->last_key, key->buf, key->len); +reftable_buf_reset (&w->last_key); +reftable_buf_add (&w->last_key, key->buf, key->len); w->entries++;return 0 ; }
@@ -98,8 +98,8 @@ uint8_t block_writer_type(struct block_writer *bw)empty key. */ int block_writer_add (struct block_writer *w,struct reftable_record *rec) { -struct strbuf empty =STRBUF_INIT ; -struct strbuf last = +struct reftable_buf empty =REFTABLE_BUF_INIT ; +struct reftable_buf last = w->entries % w->restart_interval ==0 ? empty : w->last_key;struct string_view out = { .buf = w->buf + w->next,
@@ -109,7 +109,7 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)struct string_view start = out;int is_restart =0 ; -struct strbuf key =STRBUF_INIT ; +struct reftable_buf key =REFTABLE_BUF_INIT ;int n =0 ;int err =-1 ;
@@ -133,7 +133,7 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec) err =block_writer_register_restart (w, start.len - out.len, is_restart, &key); done: -strbuf_release (&key); +reftable_buf_release (&key);return err; }
@@ -325,7 +325,7 @@ uint8_t block_reader_type(const struct block_reader *r)return r->block.data[r->header_off]; } -int block_reader_first_key (const struct block_reader *br,struct strbuf *key) +int block_reader_first_key (const struct block_reader *br,struct reftable_buf *key) {int off = br->header_off +4 , n;struct string_view in = {
@@ -334,7 +334,7 @@ int block_reader_first_key(const struct block_reader *br, struct strbuf *key) };uint8_t extra =0 ; -strbuf_reset (key); +reftable_buf_reset (key); n =reftable_decode_key (key, &extra, in);if (n <0 )
@@ -355,13 +355,13 @@ void block_iter_seek_start(struct block_iter *it, const struct block_reader *br) it->block = br->block.data; it->block_len = br->block_len; it->hash_size = br->hash_size; -strbuf_reset (&it->last_key); +reftable_buf_reset (&it->last_key); it->next_off = br->header_off +4 ; }struct restart_needle_less_args {int error; -struct strbuf needle; +struct reftable_buf needle;const struct block_reader *reader; };
@@ -433,7 +433,7 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec)void block_iter_reset (struct block_iter *it) { -strbuf_reset (&it->last_key); +reftable_buf_reset (&it->last_key); it->next_off =0 ; it->block =NULL ; it->block_len =0 ;
@@ -442,12 +442,12 @@ void block_iter_reset(struct block_iter *it)void block_iter_close (struct block_iter *it) { -strbuf_release (&it->last_key); -strbuf_release (&it->scratch); +reftable_buf_release (&it->last_key); +reftable_buf_release (&it->scratch); }int block_iter_seek_key (struct block_iter *it,const struct block_reader *br, -struct strbuf *want) +struct reftable_buf *want) {struct restart_needle_less_args args = { .needle = *want,
@@ -537,7 +537,7 @@ int block_iter_seek_key(struct block_iter *it, const struct block_reader *br,* with themselves. */ reftable_record_key (&rec, &it->last_key); -if (strbuf_cmp (&it->last_key, want) >=0 ) { +if (reftable_buf_cmp (&it->last_key, want) >=0 ) { it->next_off = prev_off; goto done; }
@@ -554,7 +554,7 @@ void block_writer_release(struct block_writer *bw)REFTABLE_FREE_AND_NULL (bw->zstream);REFTABLE_FREE_AND_NULL (bw->restarts);REFTABLE_FREE_AND_NULL (bw->compressed); -strbuf_release (&bw->last_key); +reftable_buf_release (&bw->last_key);/* the block is not owned. */ }
reftable/block.h +7 -7
@@ -38,7 +38,7 @@ struct block_writer {uint32_t restart_len;uint32_t restart_cap; -struct strbuf last_key; +struct reftable_buf last_key;int entries; };
@@ -98,7 +98,7 @@ void block_reader_release(struct block_reader *br);uint8_t block_reader_type (const struct block_reader *r);/* Decodes the first key in the block */ -int block_reader_first_key (const struct block_reader *br,struct strbuf *key); +int block_reader_first_key (const struct block_reader *br,struct reftable_buf *key);/* Iterate over entries in a block */ struct block_iter {
@@ -109,13 +109,13 @@ struct block_iter {int hash_size;/* key for last entry we read. */ -struct strbuf last_key; -struct strbuf scratch; +struct reftable_buf last_key; +struct reftable_buf scratch; };#define BLOCK_ITER_INIT { \ - .last_key = STRBUF_INIT, \ - .scratch = STRBUF_INIT, \ + .last_key = REFTABLE_BUF_INIT, \ + .scratch = REFTABLE_BUF_INIT, \ }/* Position `it` at start of the block */
@@ -123,7 +123,7 @@ void block_iter_seek_start(struct block_iter *it, const struct block_reader *br)/* Position `it` to the `want` key in the block */ int block_iter_seek_key (struct block_iter *it,const struct block_reader *br, -struct strbuf *want); +struct reftable_buf *want);/* return < 0 for error, 0 for OK, > 0 for EOF. */ int block_iter_next (struct block_iter *it,struct reftable_record *rec);
reftable/blocksource.c +3 -3
@@ -27,7 +27,7 @@ static void strbuf_close(void *b UNUSED)static int strbuf_read_block (void *v,struct reftable_block *dest,uint64_t off,uint32_t size) { -struct strbuf *b = v; +struct reftable_buf *b = v;assert (off + size <= b->len);REFTABLE_CALLOC_ARRAY (dest->data, size);if (!dest->data)
@@ -39,7 +39,7 @@ static int strbuf_read_block(void *v, struct reftable_block *dest, uint64_t off,static uint64_t strbuf_size (void *b) { -return ((struct strbuf *)b)->len; +return ((struct reftable_buf *)b)->len; }static struct reftable_block_source_vtable strbuf_vtable = {
@@ -50,7 +50,7 @@ static struct reftable_block_source_vtable strbuf_vtable = { };void block_source_from_strbuf (struct reftable_block_source *bs, -struct strbuf *buf) +struct reftable_buf *buf) {assert (!bs->ops); bs->ops = &strbuf_vtable;
reftable/blocksource.h +2 -1
@@ -12,9 +12,10 @@ license that can be found in the LICENSE file or at#include "system.h" struct reftable_block_source ; +struct reftable_buf ;/* Create an in-memory block source for reading reftables */ void block_source_from_strbuf (struct reftable_block_source *bs, -struct strbuf *buf); +struct reftable_buf *buf);#endif
reftable/iter.c +3 -3
@@ -55,7 +55,7 @@ void iterator_set_empty(struct reftable_iterator *it)static void filtering_ref_iterator_close (void *iter_arg) {struct filtering_ref_iterator *fri = iter_arg; -strbuf_release (&fri->oid); +reftable_buf_release (&fri->oid);reftable_iterator_destroy (&fri->it); }
@@ -115,7 +115,7 @@ static void indexed_table_ref_iter_close(void *p)block_iter_close (&it->cur);reftable_block_done (&it->block_reader.block);reftable_free (it->offsets); -strbuf_release (&it->oid); +reftable_buf_release (&it->oid); }static int indexed_table_ref_iter_next_block (struct indexed_table_ref_iter *it)
@@ -197,7 +197,7 @@ int indexed_table_ref_iter_new(struct indexed_table_ref_iter **dest, *itr = empty; itr->r = r; -strbuf_add (&itr->oid, oid, oid_len); +reftable_buf_add (&itr->oid, oid, oid_len); itr->offsets = offsets; itr->offset_len = offset_len;
reftable/iter.h +4 -4
@@ -44,12 +44,12 @@ void iterator_set_empty(struct reftable_iterator *it);/* iterator that produces only ref records that point to `oid` */ struct filtering_ref_iterator { -struct strbuf oid; +struct reftable_buf oid;struct reftable_iterator it; };#define FILTERING_REF_ITERATOR_INIT \ { \ - .oid = STRBUF_INIT \ + .oid = REFTABLE_BUF_INIT \ } void iterator_from_filtering_ref_iterator(struct reftable_iterator *,
@@ -60,7 +60,7 @@ void iterator_from_filtering_ref_iterator(struct reftable_iterator *,*/ struct indexed_table_ref_iter {struct reftable_reader *r; -struct strbuf oid; +struct reftable_buf oid;/* mutable */ uint64_t *offsets;
@@ -75,7 +75,7 @@ struct indexed_table_ref_iter {#define INDEXED_TABLE_REF_ITER_INIT { \ .cur = BLOCK_ITER_INIT, \ - .oid = STRBUF_INIT, \ + .oid = REFTABLE_BUF_INIT, \ } void iterator_from_indexed_table_ref_iter(struct reftable_iterator *it,
reftable/reader.c +8 -8
@@ -350,8 +350,8 @@ static int table_iter_seek_start(struct table_iter *ti, uint8_t typ, int index)static int table_iter_seek_linear (struct table_iter *ti,struct reftable_record *want) { -struct strbuf want_key =STRBUF_INIT ; -struct strbuf got_key =STRBUF_INIT ; +struct reftable_buf want_key =REFTABLE_BUF_INIT ; +struct reftable_buf got_key =REFTABLE_BUF_INIT ;struct reftable_record rec;int err;
@@ -401,7 +401,7 @@ static int table_iter_seek_linear(struct table_iter *ti,if (err <0 ) goto done; -if (strbuf_cmp (&got_key, &want_key) >0 ) { +if (reftable_buf_cmp (&got_key, &want_key) >0 ) {table_iter_block_done (&next);break ; }
@@ -422,8 +422,8 @@ static int table_iter_seek_linear(struct table_iter *ti, done:reftable_record_release (&rec); -strbuf_release (&want_key); -strbuf_release (&got_key); +reftable_buf_release (&want_key); +reftable_buf_release (&got_key);return err; }
@@ -431,11 +431,11 @@ static int table_iter_seek_indexed(struct table_iter *ti, struct reftable_record *rec) {struct reftable_record want_index = { - .type =BLOCK_TYPE_INDEX , .u.idx = { .last_key =STRBUF_INIT } + .type =BLOCK_TYPE_INDEX , .u.idx = { .last_key =REFTABLE_BUF_INIT } };struct reftable_record index_result = { .type =BLOCK_TYPE_INDEX , - .u.idx = { .last_key =STRBUF_INIT }, + .u.idx = { .last_key =REFTABLE_BUF_INIT }, };int err;
@@ -765,7 +765,7 @@ static int reftable_reader_refs_for_unindexed(struct reftable_reader *r, } *filter = empty; -strbuf_add (&filter->oid, oid, oid_len); +reftable_buf_add (&filter->oid, oid, oid_len);iterator_from_table_iter (&filter->it, ti);iterator_from_filtering_ref_iterator (it, filter);
reftable/record.c +40 -40
235 lines not on this page · raw message
reftable/record.h +11 -10
@@ -9,6 +9,7 @@ license that can be found in the LICENSE file or at#ifndef RECORD_H #define RECORD_H +#include "basics.h" #include "system.h" #include <stdint.h>
@@ -38,8 +39,8 @@ int put_var_int(struct string_view *dest, uint64_t val);/* Methods for records. */ struct reftable_record_vtable { -/* encode the key of to a uint8_t strbuf. */ -void (*key)(const void *rec,struct strbuf *dest); +/* encode the key of to a uint8_t reftable_buf. */ +void (*key)(const void *rec,struct reftable_buf *dest);/* The record type of ('r' for ref). */ uint8_t type;
@@ -54,9 +55,9 @@ struct reftable_record_vtable {int (*encode)(const void *rec,struct string_view dest,int hash_size);/* decode data from `src` into the record. */ -int (*decode)(void *rec,struct strbuf key,uint8_t extra, +int (*decode)(void *rec,struct reftable_buf key,uint8_t extra,struct string_view src,int hash_size, -struct strbuf *scratch); +struct reftable_buf *scratch);/* deallocate and null the record. */ void (*release)(void *rec);
@@ -83,7 +84,7 @@ int reftable_is_block_type(uint8_t typ);/* Encode `key` into `dest`. Sets `is_restart` to indicate a restart. Returns * number of bytes written. */ int reftable_encode_key (int *is_restart,struct string_view dest, -struct strbuf prev_key,struct strbuf key, +struct reftable_buf prev_key,struct reftable_buf key,uint8_t extra);/* Decode a record's key lengths. */
@@ -96,13 +97,13 @@ int reftable_decode_keylen(struct string_view in,* Decode into `last_key` and `extra` from `in`. `last_key` is expected to * contain the decoded key of the preceding record, if any. */ -int reftable_decode_key (struct strbuf *last_key,uint8_t *extra, +int reftable_decode_key (struct reftable_buf *last_key,uint8_t *extra,struct string_view in);/* reftable_index_record are used internally to speed up lookups. */ struct reftable_index_record {uint64_t offset;/* Offset of block */ -struct strbuf last_key;/* Last key of the block. */ +struct reftable_buf last_key;/* Last key of the block. */ };/* reftable_obj_record stores an object ID => ref mapping. */
@@ -136,15 +137,15 @@ void reftable_record_init(struct reftable_record *rec, uint8_t typ);/* see struct record_vtable */ int reftable_record_cmp (struct reftable_record *a,struct reftable_record *b);int reftable_record_equal (struct reftable_record *a,struct reftable_record *b,int hash_size); -void reftable_record_key (struct reftable_record *rec,struct strbuf *dest); +void reftable_record_key (struct reftable_record *rec,struct reftable_buf *dest);int reftable_record_copy_from (struct reftable_record *rec,struct reftable_record *src,int hash_size);uint8_t reftable_record_val_type (struct reftable_record *rec);int reftable_record_encode (struct reftable_record *rec,struct string_view dest,int hash_size); -int reftable_record_decode (struct reftable_record *rec,struct strbuf key, +int reftable_record_decode (struct reftable_record *rec,struct reftable_buf key,uint8_t extra,struct string_view src, -int hash_size,struct strbuf *scratch); +int hash_size,struct reftable_buf *scratch);int reftable_record_is_deletion (struct reftable_record *rec);static inline uint8_t reftable_record_type (struct reftable_record *rec)
reftable/stack.c +60 -60
305 lines not on this page · raw message
reftable/system.h +0 -1
@@ -13,7 +13,6 @@ license that can be found in the LICENSE file or at#include "git-compat-util.h" #include "lockfile.h" -#include "strbuf.h" #include "tempfile.h" #include "hash.h" /* hash ID, sizes.*/ #include "dir.h" /* remove_dir_recursively, for tests.*/
reftable/writer.c +33 -33
219 lines not on this page · raw message
reftable/writer.h +1 -1
@@ -19,7 +19,7 @@ struct reftable_writer {int (*flush)(void *);void *write_arg;int pending_padding; -struct strbuf last_key; +struct reftable_buf last_key;/* offset of next block to write. */ uint64_t next;
t/unit-tests/lib-reftable.c +2 -2
@@ -19,7 +19,7 @@ static int strbuf_writer_flush(void *arg UNUSED)return 0 ; } -struct reftable_writer *t_reftable_strbuf_writer (struct strbuf *buf, +struct reftable_writer *t_reftable_strbuf_writer (struct reftable_buf *buf,struct reftable_write_options *opts) {struct reftable_writer *writer;
@@ -29,7 +29,7 @@ struct reftable_writer *t_reftable_strbuf_writer(struct strbuf *buf,return writer; } -void t_reftable_write_to_buf(struct strbuf *buf, +void t_reftable_write_to_buf(struct reftable_buf *buf, struct reftable_ref_record *refs, size_t nrefs, struct reftable_log_record *logs,
t/unit-tests/lib-reftable.h +4 -3
@@ -2,15 +2,16 @@#define LIB_REFTABLE_H #include "git-compat-util.h" -#include "strbuf.h" #include "reftable/reftable-writer.h" +struct reftable_buf ; +void t_reftable_set_hash (uint8_t *p,int i,uint32_t id); -struct reftable_writer *t_reftable_strbuf_writer (struct strbuf *buf, +struct reftable_writer *t_reftable_strbuf_writer (struct reftable_buf *buf,struct reftable_write_options *opts); -void t_reftable_write_to_buf(struct strbuf *buf, +void t_reftable_write_to_buf(struct reftable_buf *buf, struct reftable_ref_record *refs, size_t nrecords, struct reftable_log_record *logs,
t/unit-tests/t-reftable-basics.c +8 -8
@@ -99,8 +99,8 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED) } if_test ("common_prefix_size works") { -struct strbuf a =STRBUF_INIT ; -struct strbuf b =STRBUF_INIT ; +struct reftable_buf a =REFTABLE_BUF_INIT ; +struct reftable_buf b =REFTABLE_BUF_INIT ;struct {const char *a, *b;int want;
@@ -113,14 +113,14 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED) };for (size_t i =0 ; i <ARRAY_SIZE (cases); i++) { -strbuf_addstr (&a, cases[i].a); -strbuf_addstr (&b, cases[i].b); +reftable_buf_addstr (&a, cases[i].a); +reftable_buf_addstr (&b, cases[i].b); check_int(common_prefix_size(&a, &b), ==, cases[i].want); -strbuf_reset (&a); -strbuf_reset (&b); +reftable_buf_reset (&a); +reftable_buf_reset (&b); } -strbuf_release (&a); -strbuf_release (&b); +reftable_buf_release (&a); +reftable_buf_release (&b); } if_test ("put_be24 and get_be24 work") {
t/unit-tests/t-reftable-block.c +21 -21
129 lines not on this page · raw message
t/unit-tests/t-reftable-merged.c +13 -13
@@ -20,7 +20,7 @@ static struct reftable_merged_table * merged_table_from_records(struct reftable_ref_record **refs, struct reftable_block_source **source, struct reftable_reader ***readers, const size_t *sizes, - struct strbuf *buf, const size_t n) + struct reftable_buf *buf, const size_t n) { struct reftable_merged_table *mt = NULL; struct reftable_write_options opts = {
@@ -75,7 +75,7 @@ static void t_merged_single_record(void)struct reftable_ref_record *refs[] = { r1, r2, r3 };size_t sizes[] = {ARRAY_SIZE (r1),ARRAY_SIZE (r2),ARRAY_SIZE (r3) }; -struct strbuf bufs[3 ] = {STRBUF_INIT ,STRBUF_INIT ,STRBUF_INIT }; +struct reftable_buf bufs[3 ] = {REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT };struct reftable_block_source *bs =NULL ;struct reftable_reader **readers =NULL ; struct reftable_merged_table *mt =
@@ -97,7 +97,7 @@ static void t_merged_single_record(void)readers_destroy (readers,3 );reftable_merged_table_free (mt);for (size_t i =0 ; i <ARRAY_SIZE (bufs); i++) -strbuf_release (&bufs[i]); +reftable_buf_release (&bufs[i]);reftable_free (bs); }
@@ -152,7 +152,7 @@ static void t_merged_refs(void)struct reftable_ref_record *refs[] = { r1, r2, r3 };size_t sizes[3 ] = {ARRAY_SIZE (r1),ARRAY_SIZE (r2),ARRAY_SIZE (r3) }; -struct strbuf bufs[3 ] = {STRBUF_INIT ,STRBUF_INIT ,STRBUF_INIT }; +struct reftable_buf bufs[3 ] = {REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT };struct reftable_block_source *bs =NULL ;struct reftable_reader **readers =NULL ; struct reftable_merged_table *mt =
@@ -192,7 +192,7 @@ static void t_merged_refs(void)reftable_free (out);for (i =0 ; i <3 ; i++) -strbuf_release (&bufs[i]); +reftable_buf_release (&bufs[i]);readers_destroy (readers,3 );reftable_merged_table_free (mt);reftable_free (bs);
@@ -234,8 +234,8 @@ static void t_merged_seek_multiple_times(void)size_t sizes[] = {ARRAY_SIZE (r1),ARRAY_SIZE (r2), }; -struct strbuf bufs[] = { -STRBUF_INIT ,STRBUF_INIT , +struct reftable_buf bufs[] = { +REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT , };struct reftable_block_source *sources =NULL ;struct reftable_reader **readers =NULL ;
@@ -265,7 +265,7 @@ static void t_merged_seek_multiple_times(void) }for (size_t i =0 ; i <ARRAY_SIZE (bufs); i++) -strbuf_release (&bufs[i]); +reftable_buf_release (&bufs[i]);readers_destroy (readers,ARRAY_SIZE (refs));reftable_ref_record_release (&rec);reftable_iterator_destroy (&it);
@@ -277,7 +277,7 @@ static struct reftable_merged_table * merged_table_from_log_records(struct reftable_log_record **logs, struct reftable_block_source **source, struct reftable_reader ***readers, const size_t *sizes, - struct strbuf *buf, const size_t n) + struct reftable_buf *buf, const size_t n) { struct reftable_merged_table *mt = NULL; struct reftable_write_options opts = {
@@ -361,7 +361,7 @@ static void t_merged_logs(void)struct reftable_log_record *logs[] = { r1, r2, r3 };size_t sizes[3 ] = {ARRAY_SIZE (r1),ARRAY_SIZE (r2),ARRAY_SIZE (r3) }; -struct strbuf bufs[3 ] = {STRBUF_INIT ,STRBUF_INIT ,STRBUF_INIT }; +struct reftable_buf bufs[3 ] = {REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT ,REFTABLE_BUF_INIT };struct reftable_block_source *bs =NULL ;struct reftable_reader **readers =NULL ; struct reftable_merged_table *mt = merged_table_from_log_records(
@@ -412,7 +412,7 @@ static void t_merged_logs(void)reftable_free (out);for (i =0 ; i <3 ; i++) -strbuf_release (&bufs[i]); +reftable_buf_release (&bufs[i]);readers_destroy (readers,3 );reftable_merged_table_free (mt);reftable_free (bs);
@@ -421,7 +421,7 @@ static void t_merged_logs(void)static void t_default_write_opts (void ) {struct reftable_write_options opts = {0 }; -struct strbuf buf =STRBUF_INIT ; +struct reftable_buf buf =REFTABLE_BUF_INIT ;struct reftable_writer *w =t_reftable_strbuf_writer (&buf, &opts);struct reftable_ref_record rec = { .refname = (char *)"master" ,
@@ -457,7 +457,7 @@ static void t_default_write_opts(void)reftable_reader_decref (rd);reftable_merged_table_free (merged); -strbuf_release (&buf); +reftable_buf_release (&buf); }
t/unit-tests/t-reftable-reader.c +4 -4
@@ -16,7 +16,7 @@ static int t_reader_seek_once(void)struct reftable_ref_record ref = {0 };struct reftable_iterator it = {0 };struct reftable_reader *reader; -struct strbuf buf =STRBUF_INIT ; +struct reftable_buf buf =REFTABLE_BUF_INIT ;int ret;t_reftable_write_to_buf (&buf, records,ARRAY_SIZE (records),NULL ,0 ,NULL );
@@ -40,7 +40,7 @@ static int t_reader_seek_once(void)reftable_ref_record_release (&ref);reftable_iterator_destroy (&it);reftable_reader_decref (reader); -strbuf_release (&buf); +reftable_buf_release (&buf);return 0 ; }
@@ -57,7 +57,7 @@ static int t_reader_reseek(void)struct reftable_ref_record ref = {0 };struct reftable_iterator it = {0 };struct reftable_reader *reader; -struct strbuf buf =STRBUF_INIT ; +struct reftable_buf buf =REFTABLE_BUF_INIT ;int ret;t_reftable_write_to_buf (&buf, records,ARRAY_SIZE (records),NULL ,0 ,NULL );
@@ -84,7 +84,7 @@ static int t_reader_reseek(void)reftable_ref_record_release (&ref);reftable_iterator_destroy (&it);reftable_reader_decref (reader); -strbuf_release (&buf); +reftable_buf_release (&buf);return 0 ; }
t/unit-tests/t-reftable-readwrite.c +46 -46
328 lines not on this page · raw message
t/unit-tests/t-reftable-record.c +37 -37
201 lines not on this page · raw message
t/unit-tests/t-reftable-stack.c +45 -45
214 lines not on this page · raw message
Signature · 1 line
2.47.0.72.gef8ce8f3d4.dirty
PATCH v3 5/10 reftable/blocksource: adapt interface name
In reply to Patrick SteinhardtAdapt the name of the strbuf block source to no longer relate to this interface, but instead to the reftable_buf interface.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/blocksource.c | 26 +++++++++++++------------- reftable/blocksource.h | 4 ++-- t/unit-tests/t-reftable-block.c | 8 ++++---- t/unit-tests/t-reftable-merged.c | 6 +++--- t/unit-tests/t-reftable-reader.c | 4 ++-- t/unit-tests/t-reftable-readwrite.c | 24 ++++++++++++------------ 6 files changed, 36 insertions(+), 36 deletions(-)
reftable/blocksource.c +13 -13
@@ -13,19 +13,19 @@ license that can be found in the LICENSE file or at#include "reftable-blocksource.h" #include "reftable-error.h" -static void strbuf_return_block(void *b UNUSED, struct reftable_block *dest) +static void reftable_buf_return_block(void *b UNUSED, struct reftable_block *dest) {if (dest->len)memset (dest->data,0xff , dest->len);reftable_free (dest->data); } -static void strbuf_close(void *b UNUSED) +static void reftable_buf_close(void *b UNUSED) { } -static int strbuf_read_block (void *v,struct reftable_block *dest,uint64_t off, -uint32_t size) +static int reftable_buf_read_block (void *v,struct reftable_block *dest, +uint64_t off,uint32_t size) {struct reftable_buf *b = v;assert (off + size <= b->len);
@@ -37,23 +37,23 @@ static int strbuf_read_block(void *v, struct reftable_block *dest, uint64_t off,return size; } -static uint64_t strbuf_size (void *b) +static uint64_t reftable_buf_size (void *b) {return ((struct reftable_buf *)b)->len; } -static struct reftable_block_source_vtable strbuf_vtable = { - .size = &strbuf_size, - .read_block = &strbuf_read_block, - .return_block = &strbuf_return_block, - .close = &strbuf_close, +static struct reftable_block_source_vtable reftable_buf_vtable = { + .size = &reftable_buf_size, + .read_block = &reftable_buf_read_block, + .return_block = &reftable_buf_return_block, + .close = &reftable_buf_close, }; -void block_source_from_strbuf (struct reftable_block_source *bs, -struct reftable_buf *buf) +void block_source_from_buf (struct reftable_block_source *bs, +struct reftable_buf *buf) {assert (!bs->ops); - bs->ops = &strbuf_vtable; + bs->ops = &reftable_buf_vtable; bs->arg = buf; }
reftable/blocksource.h +2 -2
@@ -15,7 +15,7 @@ struct reftable_block_source;struct reftable_buf ;/* Create an in-memory block source for reading reftables */ -void block_source_from_strbuf (struct reftable_block_source *bs, -struct reftable_buf *buf); +void block_source_from_buf (struct reftable_block_source *bs, +struct reftable_buf *buf);#endif
t/unit-tests/t-reftable-block.c +4 -4
@@ -34,7 +34,7 @@ static void t_ref_block_read_write(void)REFTABLE_CALLOC_ARRAY (block.data, block_size);check (block.data !=NULL ); block.len = block_size; -block_source_from_strbuf (&block.source ,&buf); +block_source_from_buf (&block.source ,&buf); ret =block_writer_init (&bw,BLOCK_TYPE_REF , block.data, block_size, header_off,hash_size (GIT_SHA1_FORMAT_ID ));check (!ret);
@@ -128,7 +128,7 @@ static void t_log_block_read_write(void)REFTABLE_CALLOC_ARRAY (block.data, block_size);check (block.data !=NULL ); block.len = block_size; -block_source_from_strbuf (&block.source ,&buf); +block_source_from_buf (&block.source ,&buf); ret =block_writer_init (&bw,BLOCK_TYPE_LOG , block.data, block_size, header_off,hash_size (GIT_SHA1_FORMAT_ID ));check (!ret);
@@ -218,7 +218,7 @@ static void t_obj_block_read_write(void)REFTABLE_CALLOC_ARRAY (block.data, block_size);check (block.data !=NULL ); block.len = block_size; -block_source_from_strbuf (&block.source, &buf); +block_source_from_buf (&block.source, &buf); ret =block_writer_init (&bw,BLOCK_TYPE_OBJ , block.data, block_size, header_off,hash_size (GIT_SHA1_FORMAT_ID ));check (!ret);
@@ -302,7 +302,7 @@ static void t_index_block_read_write(void)REFTABLE_CALLOC_ARRAY (block.data, block_size);check (block.data !=NULL ); block.len = block_size; -block_source_from_strbuf (&block.source, &buf); +block_source_from_buf (&block.source, &buf); ret =block_writer_init (&bw,BLOCK_TYPE_INDEX , block.data, block_size, header_off,hash_size (GIT_SHA1_FORMAT_ID ));check (!ret);
t/unit-tests/t-reftable-merged.c +3 -3
@@ -35,7 +35,7 @@ merged_table_from_records(struct reftable_ref_record **refs,for (size_t i =0 ; i < n; i++) {t_reftable_write_to_buf (&buf[i], refs[i], sizes[i],NULL ,0 , &opts); -block_source_from_strbuf (&(*source)[i], &buf[i]); +block_source_from_buf (&(*source)[i], &buf[i]); err =reftable_reader_new (&(*readers)[i], &(*source)[i],"name" );
@@ -293,7 +293,7 @@ merged_table_from_log_records(struct reftable_log_record **logs,for (size_t i =0 ; i < n; i++) {t_reftable_write_to_buf (&buf[i],NULL ,0 , logs[i], sizes[i], &opts); -block_source_from_strbuf (&(*source)[i], &buf[i]); +block_source_from_buf (&(*source)[i], &buf[i]); err =reftable_reader_new (&(*readers)[i], &(*source)[i],"name" );
@@ -442,7 +442,7 @@ static void t_default_write_opts(void)check (!err);reftable_writer_free (w); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&rd, &source,"filename" );check (!err);
t/unit-tests/t-reftable-reader.c +2 -2
@@ -20,7 +20,7 @@ static int t_reader_seek_once(void)int ret;t_reftable_write_to_buf (&buf, records,ARRAY_SIZE (records),NULL ,0 ,NULL ); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); ret =reftable_reader_new (&reader, &source,"name" );check (!ret);
@@ -61,7 +61,7 @@ static int t_reader_reseek(void)int ret;t_reftable_write_to_buf (&buf, records,ARRAY_SIZE (records),NULL ,0 ,NULL ); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); ret =reftable_reader_new (&reader, &source,"name" );check (!ret);
t/unit-tests/t-reftable-readwrite.c +12 -12
@@ -24,7 +24,7 @@ static void t_buffer(void)int n;uint8_t in[] ="hello" ;reftable_buf_add (&buf, in,sizeof (in)); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); check_int(block_source_size(&source), ==, 6); n =block_source_read_block (&source, &out,0 ,sizeof (in)); check_int(n, ==, sizeof(in));
@@ -207,7 +207,7 @@ static void t_log_write_read(void)reftable_writer_free (w); w =NULL ; -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.log" );check (!err);
@@ -298,7 +298,7 @@ static void t_log_zlib_corruption(void)/* corrupt the data. */ buf.buf[50 ] ^=0x99 ; -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.log" );check (!err);
@@ -328,7 +328,7 @@ static void t_table_read_write_sequential(void)write_table (&names, &buf,N ,256 ,GIT_SHA1_FORMAT_ID ); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.ref" );check (!err);
@@ -380,7 +380,7 @@ static void t_table_read_api(void)write_table (&names, &buf,N ,256 ,GIT_SHA1_FORMAT_ID ); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.ref" );check (!err);
@@ -416,7 +416,7 @@ static void t_table_read_write_seek(int index, int hash_id)write_table (&names, &buf,N ,256 , hash_id); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.ref" );check (!err);
@@ -538,7 +538,7 @@ static void t_table_refs_for(int indexed)reftable_writer_free (w); w =NULL ; -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.ref" );check (!err);
@@ -600,7 +600,7 @@ static void t_write_empty_table(void) check_int(buf.len, ==, header_size(1) + footer_size(1)); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&rd, &source,"filename" );check (!err);
@@ -806,7 +806,7 @@ static void t_write_multiple_indices(void) check_int(stats->obj_stats.index_offset, >, 0); check_int(stats->log_stats.index_offset, >, 0); -block_source_from_strbuf (&source, &writer_buf); +block_source_from_buf (&source, &writer_buf); err =reftable_reader_new (&reader, &source,"filename" );check (!err);
@@ -863,7 +863,7 @@ static void t_write_multi_level_index(void) stats =reftable_writer_stats (writer); check_int(stats->ref_stats.max_index_level, ==, 2); -block_source_from_strbuf (&source, &writer_buf); +block_source_from_buf (&source, &writer_buf); err =reftable_reader_new (&reader, &source,"filename" );check (!err);
@@ -889,7 +889,7 @@ static void t_corrupt_table_empty(void)struct reftable_reader *reader;int err; -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.log" ); check_int(err, ==, REFTABLE_FORMAT_ERROR); }
@@ -903,7 +903,7 @@ static void t_corrupt_table(void)int err;reftable_buf_add (&buf, zeros,sizeof (zeros)); -block_source_from_strbuf (&source, &buf); +block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.log" ); check_int(err, ==, REFTABLE_FORMAT_ERROR);
Signature · 1 line
2.47.0.72.gef8ce8f3d4.dirty
PATCH v3 6/10 t/unit-tests: check for `reftable_buf` allocation errors
In reply to Patrick SteinhardtAdapt our unit tests to check for allocations errors.
Signed-off-by Patrick Steinhardt <[email protected]>
--- t/unit-tests/t-reftable-basics.c | 4 +- t/unit-tests/t-reftable-block.c | 4 +- t/unit-tests/t-reftable-readwrite.c | 8 ++-- t/unit-tests/t-reftable-record.c | 14 +++---- t/unit-tests/t-reftable-stack.c | 58 ++++++++++++++--------------- 5 files changed, 44 insertions(+), 44 deletions(-)
t/unit-tests/t-reftable-basics.c +2 -2
@@ -113,8 +113,8 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED) };for (size_t i =0 ; i <ARRAY_SIZE (cases); i++) { -reftable_buf_addstr (&a, cases[i].a); -reftable_buf_addstr (&b, cases[i].b); +check (!reftable_buf_addstr (&a, cases[i].a)); +check (!reftable_buf_addstr (&b, cases[i].b)); check_int(common_prefix_size(&a, &b), ==, cases[i].want);reftable_buf_reset (&a);reftable_buf_reset (&b);
t/unit-tests/t-reftable-block.c +2 -2
@@ -167,7 +167,7 @@ static void t_log_block_read_write(void)for (i =0 ; i <N ; i++) {block_iter_reset (&it);reftable_buf_reset (&want); -reftable_buf_addstr (&want, recs[i].u.log.refname); +check (!reftable_buf_addstr (&want, recs[i].u.log.refname)); ret =block_iter_seek_key (&it, &br, &want); check_int(ret, ==, 0);
@@ -314,7 +314,7 @@ static void t_index_block_read_write(void)reftable_buf_init (&recs[i].u.idx.last_key); recs[i].type =BLOCK_TYPE_INDEX ; -reftable_buf_addstr (&recs[i].u.idx.last_key, buf); +check (!reftable_buf_addstr (&recs[i].u.idx.last_key, buf)); recs[i].u.idx.offset = i; ret =block_writer_add (&bw, &recs[i]);
t/unit-tests/t-reftable-readwrite.c +4 -4
@@ -23,7 +23,7 @@ static void t_buffer(void)struct reftable_block out = {0 };int n;uint8_t in[] ="hello" ; -reftable_buf_add (&buf, in,sizeof (in)); +check (!reftable_buf_add (&buf, in,sizeof (in)));block_source_from_buf (&source, &buf); check_int(block_source_size(&source), ==, 6); n =block_source_read_block (&source, &out,0 ,sizeof (in));
@@ -443,8 +443,8 @@ static void t_table_read_write_seek(int index, int hash_id)reftable_iterator_destroy (&it); } -reftable_buf_addstr (&pastLast, names[N -1 ]); -reftable_buf_addstr (&pastLast,"/" ); +check (!reftable_buf_addstr (&pastLast, names[N -1 ])); +check (!reftable_buf_addstr (&pastLast,"/" )); err =reftable_reader_init_ref_iterator (reader, &it);check (!err);
@@ -901,7 +901,7 @@ static void t_corrupt_table(void)struct reftable_block_source source = {0 };struct reftable_reader *reader;int err; -reftable_buf_add (&buf, zeros,sizeof (zeros)); +check (!reftable_buf_add (&buf, zeros,sizeof (zeros)));block_source_from_buf (&source, &buf); err =reftable_reader_new (&reader, &source,"file.log" );
t/unit-tests/t-reftable-record.c +7 -7
@@ -335,14 +335,14 @@ static void t_key_roundtrip(void)int n, m;uint8_t rt_extra; -reftable_buf_addstr (&last_key,"refs/heads/master" ); -reftable_buf_addstr (&key,"refs/tags/bla" ); +check (!reftable_buf_addstr (&last_key,"refs/heads/master" )); +check (!reftable_buf_addstr (&key,"refs/tags/bla" )); extra =6 ; n =reftable_encode_key (&restart, dest, last_key, key, extra);check (!restart); check_int(n, >, 0); -reftable_buf_addstr (&roundtrip,"refs/heads/master" ); +check (!reftable_buf_addstr (&roundtrip,"refs/heads/master" )); m =reftable_decode_key (&roundtrip, &rt_extra, dest); check_int(n, ==, m);check (!reftable_buf_cmp (&key, &roundtrip));
@@ -469,9 +469,9 @@ static void t_reftable_index_record_comparison(void) .u.idx.last_key = REFTABLE_BUF_INIT, }, }; -reftable_buf_addstr (&in[0 ].u.idx.last_key,"refs/heads/master" ); -reftable_buf_addstr (&in[1 ].u.idx.last_key,"refs/heads/master" ); -reftable_buf_addstr (&in[2 ].u.idx.last_key,"refs/heads/branch" ); +check (!reftable_buf_addstr (&in[0 ].u.idx.last_key,"refs/heads/master" )); +check (!reftable_buf_addstr (&in[1 ].u.idx.last_key,"refs/heads/master" )); +check (!reftable_buf_addstr (&in[2 ].u.idx.last_key,"refs/heads/branch" ));check (!reftable_record_equal (&in[0 ], &in[1 ],GIT_SHA1_RAWSZ ));check (!reftable_record_cmp (&in[0 ], &in[1 ]));
@@ -510,7 +510,7 @@ static void t_reftable_index_record_roundtrip(void)int n, m;uint8_t extra; -reftable_buf_addstr (&in.u.idx.last_key,"refs/heads/master" ); +check (!reftable_buf_addstr (&in.u.idx.last_key,"refs/heads/master" ));reftable_record_key (&in, &key);t_copy (&in);
t/unit-tests/t-reftable-stack.c +29 -29
106 lines not on this page · raw message
Signature · 1 line
2.47.0.72.gef8ce8f3d4.dirty
PATCH v3 7/10 reftable/stack: adapt `format_name()` to handle allocation failures
In reply to Patrick SteinhardtThe format_name() function cannot pass any errors to the caller as it has a void return type. Adapt it and its callers such that we can handle errors and start handling allocation failures.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/stack.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-)
reftable/stack.c +19 -9
@@ -623,14 +623,14 @@ int reftable_stack_add(struct reftable_stack *st,return 0 ; } -static void format_name (struct reftable_buf *dest,uint64_t min,uint64_t max) +static int format_name (struct reftable_buf *dest,uint64_t min,uint64_t max) {char buf[100 ];uint32_t rnd = (uint32_t )git_rand ();snprintf (buf,sizeof (buf),"0x%012" PRIx64"-0x%012" PRIx64"-%08x" , min, max, rnd);reftable_buf_reset (dest); -reftable_buf_addstr (dest, buf); +return reftable_buf_addstr (dest, buf); }struct reftable_addition {
@@ -846,7 +846,10 @@ int reftable_addition_add(struct reftable_addition *add,int tab_fd;reftable_buf_reset (&next_name); -format_name (&next_name, add->next_update_index, add->next_update_index); + + err =format_name (&next_name, add->next_update_index, add->next_update_index); +if (err <0 ) + goto done;stack_filename (&temp_tab_file_name, add->stack, next_name.buf);reftable_buf_addstr (&temp_tab_file_name,".temp.XXXXXX" );
@@ -893,7 +896,9 @@ int reftable_addition_add(struct reftable_addition *add, goto done; } -format_name (&next_name, wr->min_update_index, wr->max_update_index); + err =format_name (&next_name, wr->min_update_index, wr->max_update_index); +if (err <0 ) + goto done;reftable_buf_addstr (&next_name,".ref" );stack_filename (&tab_file_name, add->stack, next_name.buf);
@@ -944,9 +949,11 @@ static int stack_compact_locked(struct reftable_stack *st,struct tempfile *tab_file;int tab_fd, err =0 ; -format_name (&next_name, -reftable_reader_min_update_index (st->readers[first]), -reftable_reader_max_update_index (st->readers[last])); + err =format_name (&next_name,reftable_reader_min_update_index (st->readers[first]), +reftable_reader_max_update_index (st->readers[last])); +if (err <0 ) + goto done; +stack_filename (&tab_file_path, st, next_name.buf);reftable_buf_addstr (&tab_file_path,".temp.XXXXXX" );
@@ -1370,8 +1377,11 @@ static int stack_compact_range(struct reftable_stack *st,* it into place now. */ if (!is_empty_table) { -format_name (&new_table_name, st->readers[first]->min_update_index, - st->readers[last]->max_update_index); + err =format_name (&new_table_name, st->readers[first]->min_update_index, + st->readers[last]->max_update_index); +if (err <0 ) + goto done; +reftable_buf_addstr (&new_table_name,".ref" );stack_filename (&new_table_path, st, new_table_name.buf);
Signature · 1 line
2.47.0.72.gef8ce8f3d4.dirty
PATCH v3 8/10 reftable/record: adapt `reftable_record_key()` to handle allocation failures
In reply to Patrick SteinhardtThe reftable_record_key() function cannot pass any errors to the caller as it has a void return type. Adapt it and its callers such that we can handle errors and start handling allocation failures.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/block.c | 20 +++++++++++++++----- reftable/reader.c | 8 ++++++-- reftable/record.c | 32 ++++++++++++++++++++------------ reftable/record.h | 4 ++-- reftable/writer.c | 5 ++++- 5 files changed, 47 insertions(+), 22 deletions(-)
reftable/block.c +15 -5
@@ -111,9 +111,12 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)int is_restart =0 ;struct reftable_buf key =REFTABLE_BUF_INIT ;int n =0 ; -int err =-1 ; +int err; + + err =reftable_record_key (rec, &key); +if (err <0 ) + goto done; -reftable_record_key (rec, &key);if (!key.len) { err =REFTABLE_API_ERROR ; goto done;
@@ -121,13 +124,17 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec) n =reftable_encode_key (&is_restart, out, last, key,reftable_record_val_type (rec)); -if (n <0 ) +if (n <0 ) { + err =-1 ; goto done; + }string_view_consume (&out, n); n =reftable_record_encode (rec, out, w->hash_size); -if (n <0 ) +if (n <0 ) { + err =-1 ; goto done; + }string_view_consume (&out, n); err = block_writer_register_restart(w, start.len - out.len, is_restart,
@@ -522,6 +529,10 @@ int block_iter_seek_key(struct block_iter *it, const struct block_reader *br, goto done; } + err =reftable_record_key (&rec, &it->last_key); +if (err <0 ) + goto done; +/* * Check whether the current key is greater or equal to the * sought-after key. In case it is greater we know that the
@@ -536,7 +547,6 @@ int block_iter_seek_key(struct block_iter *it, const struct block_reader *br,* to `last_key` now, and naturally all keys share a prefix * with themselves. */ -reftable_record_key (&rec, &it->last_key);if (reftable_buf_cmp (&it->last_key, want) >=0 ) { it->next_off = prev_off; goto done;
reftable/reader.c +6 -2
@@ -356,7 +356,9 @@ static int table_iter_seek_linear(struct table_iter *ti,int err;reftable_record_init (&rec,reftable_record_type (want)); -reftable_record_key (want, &want_key); + err =reftable_record_key (want, &want_key); +if (err <0 ) + goto done;/* * First we need to locate the block that must contain our record. To
@@ -439,7 +441,9 @@ static int table_iter_seek_indexed(struct table_iter *ti, };int err; -reftable_record_key (rec, &want_index.u.idx.last_key); + err =reftable_record_key (rec, &want_index.u.idx.last_key); +if (err <0 ) + goto done;/* * The index may consist of multiple levels, where each level may have
reftable/record.c +20 -12
@@ -207,12 +207,12 @@ int reftable_decode_key(struct reftable_buf *last_key, uint8_t *extra,return start_len - in.len; } -static void reftable_ref_record_key (const void *r,struct reftable_buf *dest) +static int reftable_ref_record_key (const void *r,struct reftable_buf *dest) {const struct reftable_ref_record *rec = (const struct reftable_ref_record *)r;reftable_buf_reset (dest); -reftable_buf_addstr (dest, rec->refname); +return reftable_buf_addstr (dest, rec->refname); } static int reftable_ref_record_copy_from(void *rec, const void *src_rec,
@@ -465,12 +465,12 @@ static struct reftable_record_vtable reftable_ref_record_vtable = { .cmp = &reftable_ref_record_cmp_void, }; -static void reftable_obj_record_key (const void *r,struct reftable_buf *dest) +static int reftable_obj_record_key (const void *r,struct reftable_buf *dest) {const struct reftable_obj_record *rec = (const struct reftable_obj_record *)r;reftable_buf_reset (dest); -reftable_buf_add (dest, rec->hash_prefix, rec->hash_prefix_len); +return reftable_buf_add (dest, rec->hash_prefix, rec->hash_prefix_len); }static void reftable_obj_record_release (void *rec)
@@ -664,19 +664,27 @@ static struct reftable_record_vtable reftable_obj_record_vtable = { .cmp = &reftable_obj_record_cmp_void, }; -static void reftable_log_record_key (const void *r,struct reftable_buf *dest) +static int reftable_log_record_key (const void *r,struct reftable_buf *dest) {const struct reftable_log_record *rec = (const struct reftable_log_record *)r; -int len =strlen (rec->refname); +int len =strlen (rec->refname), err;uint8_t i64[8 ];uint64_t ts =0 ; +reftable_buf_reset (dest); -reftable_buf_add (dest, (uint8_t *)rec->refname, len +1 ); + err =reftable_buf_add (dest, (uint8_t *)rec->refname, len +1 ); +if (err <0 ) +return err; ts = (~ts) - rec->update_index;put_be64 (&i64[0 ], ts); -reftable_buf_add (dest, i64,sizeof (i64)); + + err =reftable_buf_add (dest, i64,sizeof (i64)); +if (err <0 ) +return err; + +return 0 ; } static int reftable_log_record_copy_from(void *rec, const void *src_rec,
@@ -1027,11 +1035,11 @@ static struct reftable_record_vtable reftable_log_record_vtable = { .cmp = &reftable_log_record_cmp_void, }; -static void reftable_index_record_key (const void *r,struct reftable_buf *dest) +static int reftable_index_record_key (const void *r,struct reftable_buf *dest) {const struct reftable_index_record *rec = r;reftable_buf_reset (dest); -reftable_buf_add (dest, rec->last_key.buf, rec->last_key.len); +return reftable_buf_add (dest, rec->last_key.buf, rec->last_key.len); } static int reftable_index_record_copy_from(void *rec, const void *src_rec,
@@ -1124,9 +1132,9 @@ static struct reftable_record_vtable reftable_index_record_vtable = { .cmp = &reftable_index_record_cmp, }; -void reftable_record_key (struct reftable_record *rec,struct reftable_buf *dest) +int reftable_record_key (struct reftable_record *rec,struct reftable_buf *dest) { -reftable_record_vtable (rec)->key (reftable_record_data (rec), dest); +return reftable_record_vtable (rec)->key (reftable_record_data (rec), dest); } int reftable_record_encode(struct reftable_record *rec, struct string_view dest,
reftable/record.h +2 -2
@@ -40,7 +40,7 @@ int put_var_int(struct string_view *dest, uint64_t val);/* Methods for records. */ struct reftable_record_vtable {/* encode the key of to a uint8_t reftable_buf. */ -void (*key)(const void *rec,struct reftable_buf *dest); +int (*key)(const void *rec,struct reftable_buf *dest);/* The record type of ('r' for ref). */ uint8_t type;
@@ -137,7 +137,7 @@ void reftable_record_init(struct reftable_record *rec, uint8_t typ);/* see struct record_vtable */ int reftable_record_cmp (struct reftable_record *a,struct reftable_record *b);int reftable_record_equal (struct reftable_record *a,struct reftable_record *b,int hash_size); -void reftable_record_key (struct reftable_record *rec,struct reftable_buf *dest); +int reftable_record_key (struct reftable_record *rec,struct reftable_buf *dest);int reftable_record_copy_from (struct reftable_record *rec,struct reftable_record *src,int hash_size);uint8_t reftable_record_val_type (struct reftable_record *rec);
reftable/writer.c +4 -1
@@ -249,7 +249,10 @@ static int writer_add_record(struct reftable_writer *w,struct reftable_buf key =REFTABLE_BUF_INIT ;int err; -reftable_record_key (rec, &key); + err =reftable_record_key (rec, &key); +if (err <0 ) + goto done; +if (reftable_buf_cmp (&w->last_key, &key) >=0 ) { err =REFTABLE_API_ERROR ; goto done;
Signature · 1 line
2.47.0.72.gef8ce8f3d4.dirty
PATCH v3 9/10 reftable/stack: adapt `stack_filename()` to handle allocation failures
In reply to Patrick SteinhardtThe stack_filename() function cannot pass any errors to the caller as it has a void return type. Adapt it and its callers such that we can handle errors and start handling allocation failures.
There are two interesting edge cases in reftable_stack_destroy() and reftable_addition_close(). Both of these are trying to tear down their respective structures, and while doing so they try to unlink some of the tables they have been keeping alive. Any earlier attempts to do that may fail on Windows because it keeps us from deleting such tables while they are still open, and thus we re-try on close. It's okay and even expected that this can fail when the tables are still open by another process, so we handle the allocation failures gracefully and just skip over any file whose name we couldn't figure out.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/stack.c | 62 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 17 deletions(-)
reftable/stack.c +45 -17
@@ -31,13 +31,16 @@ static void reftable_addition_close(struct reftable_addition *add);static int reftable_stack_reload_maybe_reuse (struct reftable_stack *st,int reuse_open); -static void stack_filename (struct reftable_buf *dest,struct reftable_stack *st, -const char *name) +static int stack_filename (struct reftable_buf *dest,struct reftable_stack *st, +const char *name) { +int err;reftable_buf_reset (dest); -reftable_buf_addstr (dest, st->reftable_dir); -reftable_buf_addstr (dest,"/" ); -reftable_buf_addstr (dest, name); +if ((err =reftable_buf_addstr (dest, st->reftable_dir)) <0 || + (err =reftable_buf_addstr (dest,"/" )) <0 || + (err =reftable_buf_addstr (dest, name)) <0 ) +return err; +return 0 ; }static ssize_t reftable_fd_write (void *arg,const void *data,size_t sz)
@@ -211,13 +214,16 @@ void reftable_stack_destroy(struct reftable_stack *st)struct reftable_buf filename =REFTABLE_BUF_INIT ;for (i =0 ; i < st->readers_len; i++) {const char *name =reader_name (st->readers[i]); +int try_unlinking =1 ; +reftable_buf_reset (&filename);if (names && !has_name (names, name)) { -stack_filename (&filename, st, name); +if (stack_filename (&filename, st, name) <0 ) + try_unlinking =0 ; }reftable_reader_decref (st->readers[i]); -if (filename.len) { +if (try_unlinking && filename.len) {/* On Windows, can only unlink after closing. */ unlink (filename.buf); }
@@ -310,7 +316,10 @@ static int reftable_stack_reload_once(struct reftable_stack *st,if (!rd) {struct reftable_block_source src = {NULL }; -stack_filename (&table_path, st, name); + + err =stack_filename (&table_path, st, name); +if (err <0 ) + goto done; err =reftable_block_source_from_file (&src, table_path.buf);
@@ -341,7 +350,11 @@ static int reftable_stack_reload_once(struct reftable_stack *st,for (i =0 ; i < cur_len; i++) {if (cur[i]) {const char *name =reader_name (cur[i]); -stack_filename (&table_path, st, name); + + err =stack_filename (&table_path, st, name); +if (err <0 ) + goto done; +reftable_reader_decref (cur[i]);unlink (table_path.buf); }
@@ -700,8 +713,8 @@ static void reftable_addition_close(struct reftable_addition *add)size_t i;for (i =0 ; i < add->new_tables_len; i++) { -stack_filename (&nm, add->stack, add->new_tables[i]); -unlink (nm.buf); +if (!stack_filename (&nm, add->stack, add->new_tables[i])) +unlink (nm.buf);reftable_free (add->new_tables[i]); add->new_tables[i] =NULL ; }
@@ -851,7 +864,9 @@ int reftable_addition_add(struct reftable_addition *add,if (err <0 ) goto done; -stack_filename (&temp_tab_file_name, add->stack, next_name.buf); + err =stack_filename (&temp_tab_file_name, add->stack, next_name.buf); +if (err <0 ) + goto done;reftable_buf_addstr (&temp_tab_file_name,".temp.XXXXXX" ); tab_file =mks_tempfile (temp_tab_file_name.buf);
@@ -900,7 +915,10 @@ int reftable_addition_add(struct reftable_addition *add,if (err <0 ) goto done;reftable_buf_addstr (&next_name,".ref" ); -stack_filename (&tab_file_name, add->stack, next_name.buf); + + err =stack_filename (&tab_file_name, add->stack, next_name.buf); +if (err <0 ) + goto done;/* On windows, this relies on rand() picking a unique destination name.
@@ -954,7 +972,9 @@ static int stack_compact_locked(struct reftable_stack *st,if (err <0 ) goto done; -stack_filename (&tab_file_path, st, next_name.buf); + err =stack_filename (&tab_file_path, st, next_name.buf); +if (err <0 ) + goto done;reftable_buf_addstr (&tab_file_path,".temp.XXXXXX" ); tab_file =mks_tempfile (tab_file_path.buf);
@@ -1174,7 +1194,9 @@ static int stack_compact_range(struct reftable_stack *st, }for (i = last +1 ; i > first; i--) { -stack_filename (&table_name, st,reader_name (st->readers[i -1 ])); + err =stack_filename (&table_name, st,reader_name (st->readers[i -1 ])); +if (err <0 ) + goto done; err =hold_lock_file_for_update (&table_locks[nlocks], table_name.buf,LOCK_NO_DEREF );
@@ -1383,7 +1405,10 @@ static int stack_compact_range(struct reftable_stack *st, goto done;reftable_buf_addstr (&new_table_name,".ref" ); -stack_filename (&new_table_path, st, new_table_name.buf); + + err =stack_filename (&new_table_path, st, new_table_name.buf); +if (err <0 ) + goto done; err =rename_tempfile (&new_table, new_table_path.buf);if (err <0 ) {
@@ -1677,7 +1702,10 @@ static void remove_maybe_stale_table(struct reftable_stack *st, uint64_t max,struct reftable_block_source src = {NULL };struct reftable_reader *rd =NULL ;struct reftable_buf table_path =REFTABLE_BUF_INIT ; -stack_filename (&table_path, st, name); + + err =stack_filename (&table_path, st, name); +if (err <0 ) + goto done; err =reftable_block_source_from_file (&src, table_path.buf);if (err <0 )
Signature · 1 line
2.47.0.72.gef8ce8f3d4.dirty
PATCH v3 10/10 reftable: handle trivial `reftable_buf` errors
In reply to Patrick SteinhardtConvert the reftable library such that we handle failures with the new reftable_buf interfaces.
Signed-off-by Patrick Steinhardt <[email protected]>
--- reftable/block.c | 9 +++++++-- reftable/iter.c | 5 ++++- reftable/reader.c | 5 ++++- reftable/record.c | 32 +++++++++++++++++++++++-------- reftable/stack.c | 49 +++++++++++++++++++++++++++++++---------------- reftable/writer.c | 48 +++++++++++++++++++++++++++++++++------------- 6 files changed, 107 insertions(+), 41 deletions(-)
reftable/block.c +7 -2
@@ -40,7 +40,9 @@ int footer_size(int version)static int block_writer_register_restart (struct block_writer *w,int n,int is_restart,struct reftable_buf *key) { -int rlen = w->restart_len; +int rlen, err; + + rlen = w->restart_len;if (rlen >=MAX_RESTARTS ) { is_restart =0 ; }
@@ -60,7 +62,10 @@ static int block_writer_register_restart(struct block_writer *w, int n, w->next += n;reftable_buf_reset (&w->last_key); -reftable_buf_add (&w->last_key, key->buf, key->len); + err =reftable_buf_add (&w->last_key, key->buf, key->len); +if (err <0 ) +return err; + w->entries++;return 0 ; }
reftable/iter.c +4 -1
@@ -197,7 +197,10 @@ int indexed_table_ref_iter_new(struct indexed_table_ref_iter **dest, *itr = empty; itr->r = r; -reftable_buf_add (&itr->oid, oid, oid_len); + + err =reftable_buf_add (&itr->oid, oid, oid_len); +if (err <0 ) + goto out; itr->offsets = offsets; itr->offset_len = offset_len;
reftable/reader.c +4 -1
@@ -769,7 +769,10 @@ static int reftable_reader_refs_for_unindexed(struct reftable_reader *r, } *filter = empty; -reftable_buf_add (&filter->oid, oid, oid_len); + err =reftable_buf_add (&filter->oid, oid, oid_len); +if (err <0 ) + goto out; +iterator_from_table_iter (&filter->it, ti);iterator_from_filtering_ref_iterator (it, filter);
reftable/record.c +24 -8
@@ -102,7 +102,9 @@ static int decode_string(struct reftable_buf *dest, struct string_view in) {int start_len = in.len;uint64_t tsize =0 ; -int n =get_var_int (&tsize, &in); +int n, err; + + n =get_var_int (&tsize, &in);if (n <=0 )return -1 ;string_view_consume (&in, n);
@@ -110,7 +112,10 @@ static int decode_string(struct reftable_buf *dest, struct string_view in)return -1 ;reftable_buf_reset (dest); -reftable_buf_add (dest, in.buf, tsize); + err =reftable_buf_add (dest, in.buf, tsize); +if (err <0 ) +return err; +string_view_consume (&in, tsize);return start_len - in.len;
@@ -189,7 +194,7 @@ int reftable_decode_key(struct reftable_buf *last_key, uint8_t *extra,int start_len = in.len;uint64_t prefix_len =0 ;uint64_t suffix_len =0 ; -int n; +int err, n; n =reftable_decode_keylen (in, &prefix_len, &suffix_len, extra);if (n <0 )
@@ -200,8 +205,14 @@ int reftable_decode_key(struct reftable_buf *last_key, uint8_t *extra, prefix_len > last_key->len)return -1 ; -reftable_buf_setlen (last_key, prefix_len); -reftable_buf_add (last_key, in.buf, suffix_len); + err =reftable_buf_setlen (last_key, prefix_len); +if (err <0 ) +return err; + + err =reftable_buf_add (last_key, in.buf, suffix_len); +if (err <0 ) +return err; +string_view_consume (&in, suffix_len);return start_len - in.len;
@@ -1047,9 +1058,12 @@ static int reftable_index_record_copy_from(void *rec, const void *src_rec, {struct reftable_index_record *dst = rec;const struct reftable_index_record *src = src_rec; +int err;reftable_buf_reset (&dst->last_key); -reftable_buf_add (&dst->last_key, src->last_key.buf, src->last_key.len); + err =reftable_buf_add (&dst->last_key, src->last_key.buf, src->last_key.len); +if (err <0 ) +return err; dst->offset = src->offset;return 0 ;
@@ -1090,10 +1104,12 @@ static int reftable_index_record_decode(void *rec, struct reftable_buf key, {struct string_view start = in;struct reftable_index_record *r = rec; -int n =0 ; +int err, n =0 ;reftable_buf_reset (&r->last_key); -reftable_buf_add (&r->last_key, key.buf, key.len); + err =reftable_buf_add (&r->last_key, key.buf, key.len); +if (err <0 ) +return err; n =get_var_int (&r->offset, &in);if (n <0 )
reftable/stack.c +33 -16
97 lines not on this page · raw message
reftable/writer.c +35 -13
96 lines not on this page · raw message
Signature · 1 line
2.47.0.72.gef8ce8f3d4.dirty
Re: [PATCH v2 03/10] reftable/basics: provide new `reftable_buf` interface
In reply to Taylor BlauQuoted 35 lines · Taylor Blau
Quoted 21 lines · Patrick Steinhardt
Quoted 20 lines · Taylor Blau
Quoted 18 lines · Eric Sunshine
Quoted 11 lines · Patrick Steinhardt
Quoted 9 lines · Taylor Blau
Review
Quoted 5 lines · Patrick Steinhardt
(quoted diff) +5
-0+/* + * Add the given bytes to the buffer. Returns 0 on success, + * REFTABLE_OUT_OF_MEMORY_ERROR on allocation failure. + */ +int reftable_buf_add(struct reftable_buf *buf, const void *data, size_t len);Is there a reason that data is a void-pointer here and not a const char *?
Quoted 1 line
Only that it emulates
strbuf_add(), which also uses a void pointer.The reason for that is because strbuf is a generic byte-array which may contain embedded NULs, and the
const void *pluslenemphasizes this property, whereasconst char *would imply a C-string with no embedded NULs.Quoted 8 lines
Quoted 3 lines
Thanks, that was the explanation I was missing. Perhaps it is worth re-stating in the commit message here to avoid confusing readers like I was when I first read Patrick's patch ;-).
Does it make sense to explicitly state how the interfaces look like though? I don't do that for the other functions either, and for most of the part I just reuse the exact same function arguments as we had with the strbuf interface.
I don't feel very strongly about it, but I had suggested it because my initial read of this patch confused me, and I had wondered if others may be similarly confused.
Quoted 8 lines
For what it's worth, I was thinking something on the order of the following added to the patch message:
Note that the
reftable_buf_add()function intentionally takes a "const void *" instead of a "const char *" (as does its strbuf counterpart,strbuf_add()) to emphasize that the buffer may contain NUL characters.But, as I said, I don't feel very strongly about it.
You know: let me amend the function documentation itself. That feels way less out of place compared to putting this info into the commit message and has the benefit that a future reader of the code will know why we have types without digging into the commit history.
Patrick
Re: [PATCH v2 03/10] reftable/basics: provide new `reftable_buf` interface
Quoted 11 lines · Patrick Steinhardt
Quoted 11 lines
I don't feel very strongly about it, but I had suggested it because my initial read of this patch confused me, and I had wondered if others may be similarly confused.
For what it's worth, I was thinking something on the order of the following added to the patch message:
Note that the
reftable_buf_add()function intentionally takes a "const void *" instead of a "const char *" (as does its strbuf counterpart,strbuf_add()) to emphasize that the buffer may contain NUL characters.
Quoted 6 lines
Quoted 1 line
But, as I said, I don't feel very strongly about it.
You know: let me amend the function documentation itself. That feels way less out of place compared to putting this info into the commit message and has the benefit that a future reader of the code will know why we have types without digging into the commit history.
Good idea, thanks.
Thanks,
Taylor
Quoted 1 line · Patrick Steinhardt
Range-diff against v2:
The range-diff looks like we're getting close here, or are already there. Do others agree that this is ready to start merging down?
Thanks,
Taylor
Quoted 8 lines · Taylor Blau
Quoted 1 line · Patrick Steinhardt
Range-diff against v2:
The range-diff looks like we're getting close here, or are already there. Do others agree that this is ready to start merging down?
Thanks,
Taylor
I had already reviewed version 1, having a look at the range-diff now, seems like it is ready to be merged!
Thanks,
Karthik
Quoted 5 lines · karthik nayak
Quoted 3 lines · Taylor Blau
Quoted 1 line · Patrick Steinhardt
Range-diff against v2:
Quoted 8 lines
Quoted 5 lines
The range-diff looks like we're getting close here, or are already there. Do others agree that this is ready to start merging down?
Thanks,
TaylorI had already reviewed version 1, having a look at the range-diff now, seems like it is ready to be merged!
Thanks, both.
Thanks,
Taylor