[v3,0/9] reftable: code style improvements

Message

Patrick Steinhardt Feb. 6, 2024, 6:35 a.m. UTC

Hi,

this is the third version of my patch series that tries to align the
reftable library's coding style to be closer to Git's own code style.

The only change compared to v2 is that I've now also converted some
calls to `reftable_malloc()` to use `REFTABLE_ALLOC_ARRAY`.

Thanks!

Patrick

Patrick Steinhardt (9):
  reftable: introduce macros to grow arrays
  reftable: introduce macros to allocate arrays
  reftable/stack: fix parameter validation when compacting range
  reftable/stack: index segments with `size_t`
  reftable/stack: use `size_t` to track stack slices during compaction
  reftable/stack: use `size_t` to track stack length
  reftable/merged: refactor seeking of records
  reftable/merged: refactor initialization of iterators
  reftable/record: improve semantics when initializing records

 reftable/basics.c          |  15 ++--
 reftable/basics.h          |  17 ++++-
 reftable/block.c           |  35 ++++-----
 reftable/block_test.c      |   2 +-
 reftable/blocksource.c     |   4 +-
 reftable/iter.c            |   3 +-
 reftable/merged.c          | 100 +++++++++++-------------
 reftable/merged_test.c     |  52 ++++++-------
 reftable/pq.c              |   8 +-
 reftable/publicbasics.c    |   3 +-
 reftable/reader.c          |  12 ++-
 reftable/readwrite_test.c  |   8 +-
 reftable/record.c          |  57 +++++---------
 reftable/record.h          |  10 +--
 reftable/record_test.c     |   8 +-
 reftable/refname.c         |   4 +-
 reftable/reftable-merged.h |   2 +-
 reftable/stack.c           | 153 +++++++++++++++++--------------------
 reftable/stack.h           |   6 +-
 reftable/stack_test.c      |   7 +-
 reftable/tree.c            |   4 +-
 reftable/writer.c          |  21 ++---
 22 files changed, 236 insertions(+), 295 deletions(-)

Range-diff against v2:
 1:  12bd721ddf =  1:  12bd721ddf reftable: introduce macros to grow arrays
 2:  2dde581a02 !  2:  95689ca7ce reftable: introduce macros to allocate arrays
    @@ Commit message
         it means that we can now provide proper overflow checks when multiplying
         the array size with the member size.
     
    -    Convert callsites of `reftable_calloc()` to the new signature, using the
    -    new macros where possible.
    +    Convert callsites of `reftable_calloc()` to the new signature and start
    +    using the new macros where possible.
     
         Signed-off-by: Patrick Steinhardt <[e-mail]>
     
    @@ reftable/basics.h: int names_length(char **names);
      #define REFTABLE_ALLOC_GROW(x, nr, alloc) \
      	do { \
     
    + ## reftable/block.c ##
    +@@ reftable/block.c: int block_writer_finish(struct block_writer *w)
    + 		int block_header_skip = 4 + w->header_off;
    + 		uLongf src_len = w->next - block_header_skip;
    + 		uLongf dest_cap = src_len * 1.001 + 12;
    ++		uint8_t *compressed;
    ++
    ++		REFTABLE_ALLOC_ARRAY(compressed, dest_cap);
    + 
    +-		uint8_t *compressed = reftable_malloc(dest_cap);
    + 		while (1) {
    + 			uLongf out_dest_len = dest_cap;
    + 			int zresult = compress2(compressed, &out_dest_len,
    +@@ reftable/block.c: int block_reader_init(struct block_reader *br, struct reftable_block *block,
    + 		uLongf dst_len = sz - block_header_skip; /* total size of dest
    + 							    buffer. */
    + 		uLongf src_len = block->len - block_header_skip;
    +-		/* Log blocks specify the *uncompressed* size in their header.
    +-		 */
    +-		uncompressed = reftable_malloc(sz);
    ++
    ++		/* Log blocks specify the *uncompressed* size in their header. */
    ++		REFTABLE_ALLOC_ARRAY(uncompressed, sz);
    + 
    + 		/* Copy over the block header verbatim. It's not compressed. */
    + 		memcpy(uncompressed, block->data, block_header_skip);
    +
      ## reftable/block_test.c ##
     @@ reftable/block_test.c: static void test_block_read_write(void)
      	int j = 0;
    @@ reftable/readwrite_test.c: static void test_table_read_write_seek_index(void)
      	uint8_t want_hash[GIT_SHA1_RAWSZ];
      
     
    + ## reftable/record.c ##
    +@@ reftable/record.c: static void reftable_obj_record_copy_from(void *rec, const void *src_rec,
    + 		(const struct reftable_obj_record *)src_rec;
    + 
    + 	reftable_obj_record_release(obj);
    +-	obj->hash_prefix = reftable_malloc(src->hash_prefix_len);
    ++
    ++	REFTABLE_ALLOC_ARRAY(obj->hash_prefix, src->hash_prefix_len);
    + 	obj->hash_prefix_len = src->hash_prefix_len;
    + 	if (src->hash_prefix_len)
    + 		memcpy(obj->hash_prefix, src->hash_prefix, obj->hash_prefix_len);
    + 
    +-	obj->offsets = reftable_malloc(src->offset_len * sizeof(uint64_t));
    ++	REFTABLE_ALLOC_ARRAY(obj->offsets, src->offset_len);
    + 	obj->offset_len = src->offset_len;
    + 	COPY_ARRAY(obj->offsets, src->offsets, src->offset_len);
    + }
    +@@ reftable/record.c: static int reftable_obj_record_decode(void *rec, struct strbuf key,
    + 	int n = 0;
    + 	uint64_t last;
    + 	int j;
    +-	r->hash_prefix = reftable_malloc(key.len);
    ++
    ++	REFTABLE_ALLOC_ARRAY(r->hash_prefix, key.len);
    + 	memcpy(r->hash_prefix, key.buf, key.len);
    + 	r->hash_prefix_len = key.len;
    + 
    +@@ reftable/record.c: static int reftable_obj_record_decode(void *rec, struct strbuf key,
    + 	if (count == 0)
    + 		return start.len - in.len;
    + 
    +-	r->offsets = reftable_malloc(count * sizeof(uint64_t));
    ++	REFTABLE_ALLOC_ARRAY(r->offsets, count);
    + 	r->offset_len = count;
    + 
    + 	n = get_var_int(&r->offsets[0], &in);
    +@@ reftable/record.c: static void reftable_log_record_copy_from(void *rec, const void *src_rec,
    + 		}
    + 
    + 		if (dst->value.update.new_hash) {
    +-			dst->value.update.new_hash = reftable_malloc(hash_size);
    ++			REFTABLE_ALLOC_ARRAY(dst->value.update.new_hash, hash_size);
    + 			memcpy(dst->value.update.new_hash,
    + 			       src->value.update.new_hash, hash_size);
    + 		}
    + 		if (dst->value.update.old_hash) {
    +-			dst->value.update.old_hash = reftable_malloc(hash_size);
    ++			REFTABLE_ALLOC_ARRAY(dst->value.update.old_hash, hash_size);
    + 			memcpy(dst->value.update.old_hash,
    + 			       src->value.update.old_hash, hash_size);
    + 		}
    +
      ## reftable/record_test.c ##
     @@ reftable/record_test.c: static void test_reftable_log_record_roundtrip(void)
      				.value_type = REFTABLE_LOG_UPDATE,
    @@ reftable/stack.c: static ssize_t reftable_fd_write(void *arg, const void *data,
      	struct strbuf list_file_name = STRBUF_INIT;
      	int err = 0;
      
    +@@ reftable/stack.c: static int fd_read_lines(int fd, char ***namesp)
    + 		goto done;
    + 	}
    + 
    +-	buf = reftable_malloc(size + 1);
    ++	REFTABLE_ALLOC_ARRAY(buf, size + 1);
    + 	if (read_in_full(fd, buf, size) != size) {
    + 		err = REFTABLE_IO_ERROR;
    + 		goto done;
     @@ reftable/stack.c: int read_lines(const char *filename, char ***namesp)
      	int err = 0;
      	if (fd < 0) {
 3:  f134702dc5 =  3:  f0e8f08884 reftable/stack: fix parameter validation when compacting range
 4:  50dac904e8 =  4:  7bcfe7b305 reftable/stack: index segments with `size_t`
 5:  a5ffbf09dd =  5:  a0867c0378 reftable/stack: use `size_t` to track stack slices during compaction
 6:  55605fb53b =  6:  29c5a54ae8 reftable/stack: use `size_t` to track stack length
 7:  80cf2fd272 =  7:  4605ad7247 reftable/merged: refactor seeking of records
 8:  8c1be2b159 =  8:  8c35968ce8 reftable/merged: refactor initialization of iterators
 9:  c39d7e30e7 =  9:  5bb2858c13 reftable/record: improve semantics when initializing records

Comments

karthik nayak Feb. 6, 2024, 9:10 a.m. UTC | #1

Hello,

Patrick Steinhardt <[e-mail]> writes:
> Hi,
>
> this is the third version of my patch series that tries to align the
> reftable library's coding style to be closer to Git's own code style.
>
> The only change compared to v2 is that I've now also converted some
> calls to `reftable_malloc()` to use `REFTABLE_ALLOC_ARRAY`.
>

The range-diff looks good to me, since this was the only change I
requested, the patch series looks good now.

Thanks!