Merge branch 'ps/reftable-sign-compare'

The reftable/ library code has been made -Wsign-compare clean.

* ps/reftable-sign-compare:
  reftable: address trivial -Wsign-compare warnings
  reftable/blocksource: adjust `read_block()` to return `ssize_t`
  reftable/blocksource: adjust type of the block length
  reftable/block: adjust type of the restart length
  reftable/block: adapt header and footer size to return a `size_t`
  reftable/basics: adjust `hash_size()` to return `uint32_t`
  reftable/basics: adjust `common_prefix_size()` to return `size_t`
  reftable/record: handle overflows when decoding varints
  reftable/record: drop unused `print` function pointer
  meson: stop disabling -Wsign-compare

meson.build

@@ -708,7 +708,6 @@ if get_option('warning_level') in ['2','3', 'everything'] and compiler.get_argum
     # These are disabled because we have these all over the place.
     '-Wno-empty-body',
     '-Wno-missing-field-initializers',
-    '-Wno-sign-compare',
   ]
     if compiler.has_argument(cflag)
       libgit_c_args += cflag

reftable/basics.c

@@ -263,18 +263,16 @@ int names_equal(const char **a, const char **b)
 	return a[i] == b[i];
 }
 
-int common_prefix_size(struct reftable_buf *a, struct reftable_buf *b)
+size_t common_prefix_size(struct reftable_buf *a, struct reftable_buf *b)
 {
-	int p = 0;
+	size_t p = 0;
-	for (; p < a->len && p < b->len; p++) {
+	for (; p < a->len && p < b->len; p++)
 		if (a->buf[p] != b->buf[p])
 			break;
-	}
-
 	return p;
 }
 
-int hash_size(enum reftable_hash id)
+uint32_t hash_size(enum reftable_hash id)
 {
 	if (!id)
 		return REFTABLE_HASH_SIZE_SHA1;

reftable/basics.h

@@ -169,9 +169,9 @@ static inline void *reftable_alloc_grow(void *p, size_t nelem, size_t elsize,
 #endif
 
 /* Find the longest shared prefix size of `a` and `b` */
-int common_prefix_size(struct reftable_buf *a, struct reftable_buf *b);
+size_t common_prefix_size(struct reftable_buf *a, struct reftable_buf *b);
 
-int hash_size(enum reftable_hash id);
+uint32_t hash_size(enum reftable_hash id);
 
 /*
  * Format IDs that identify the hash function used by a reftable. Note that

reftable/block.c

@@ -15,7 +15,7 @@ license that can be found in the LICENSE file or at
 #include "system.h"
 #include <zlib.h>
 
-int header_size(int version)
+size_t header_size(int version)
 {
 	switch (version) {
 	case 1:
@@ -26,7 +26,7 @@ int header_size(int version)
 	abort();
 }
 
-int footer_size(int version)
+size_t footer_size(int version)
 {
 	switch (version) {
 	case 1:
@@ -40,16 +40,15 @@ 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, err;
+	uint32_t rlen;
+	int err;
 
 	rlen = w->restart_len;
-	if (rlen >= MAX_RESTARTS) {
+	if (rlen >= MAX_RESTARTS)
 		is_restart = 0;
-	}
 
-	if (is_restart) {
+	if (is_restart)
 		rlen++;
-	}
 	if (2 + 3 * rlen + n > w->block_size - w->next)
 		return -1;
 	if (is_restart) {
@@ -72,7 +71,7 @@ static int block_writer_register_restart(struct block_writer *w, int n,
 }
 
 int block_writer_init(struct block_writer *bw, uint8_t typ, uint8_t *block,
-		      uint32_t block_size, uint32_t header_off, int hash_size)
+		      uint32_t block_size, uint32_t header_off, uint32_t hash_size)
 {
 	bw->block = block;
 	bw->hash_size = hash_size;
@@ -148,8 +147,7 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)
 
 int block_writer_finish(struct block_writer *w)
 {
-	int i;
+	for (uint32_t i = 0; i < w->restart_len; i++) {
-	for (i = 0; i < w->restart_len; i++) {
 		put_be24(w->block + w->next, w->restarts[i]);
 		w->next += 3;
 	}
@@ -214,7 +212,7 @@ int block_writer_finish(struct block_writer *w)
 
 int block_reader_init(struct block_reader *br, struct reftable_block *block,
 		      uint32_t header_off, uint32_t table_block_size,
-		      int hash_size)
+		      uint32_t hash_size)
 {
 	uint32_t full_block_size = table_block_size;
 	uint8_t typ = block->data[header_off];

reftable/block.h

@@ -30,7 +30,7 @@ struct block_writer {
 
 	/* How often to restart keys. */
 	uint16_t restart_interval;
-	int hash_size;
+	uint32_t hash_size;
 
 	/* Offset of next uint8_t to write. */
 	uint32_t next;
@@ -48,7 +48,7 @@ struct block_writer {
  * initializes the blockwriter to write `typ` entries, using `block` as temporary
  * storage. `block` is not owned by the block_writer. */
 int block_writer_init(struct block_writer *bw, uint8_t typ, uint8_t *block,
-		      uint32_t block_size, uint32_t header_off, int hash_size);
+		      uint32_t block_size, uint32_t header_off, uint32_t hash_size);
 
 /* returns the block type (eg. 'r' for ref records. */
 uint8_t block_writer_type(struct block_writer *bw);
@@ -72,7 +72,7 @@ struct block_reader {
 
 	/* the memory block */
 	struct reftable_block block;
-	int hash_size;
+	uint32_t hash_size;
 
 	/* Uncompressed data for log entries. */
 	z_stream *zstream;
@@ -92,7 +92,7 @@ struct block_reader {
 /* initializes a block reader. */
 int block_reader_init(struct block_reader *br, struct reftable_block *bl,
 		      uint32_t header_off, uint32_t table_block_size,
-		      int hash_size);
+		      uint32_t hash_size);
 
 void block_reader_release(struct block_reader *br);
 
@@ -108,7 +108,7 @@ struct block_iter {
 	uint32_t next_off;
 	const unsigned char *block;
 	size_t block_len;
-	int hash_size;
+	uint32_t hash_size;
 
 	/* key for last entry we read. */
 	struct reftable_buf last_key;
@@ -137,10 +137,10 @@ void block_iter_reset(struct block_iter *it);
 void block_iter_close(struct block_iter *it);
 
 /* size of file header, depending on format version */
-int header_size(int version);
+size_t header_size(int version);
 
 /* size of file footer, depending on format version */
-int footer_size(int version);
+size_t footer_size(int version);
 
 /* returns a block to its source. */
 void reftable_block_done(struct reftable_block *ret);

reftable/blocksource.c

@@ -24,7 +24,7 @@ static void reftable_buf_close(void *b UNUSED)
 {
 }
 
-static int reftable_buf_read_block(void *v, struct reftable_block *dest,
+static ssize_t reftable_buf_read_block(void *v, struct reftable_block *dest,
 				       uint64_t off, uint32_t size)
 {
 	struct reftable_buf *b = v;
@@ -78,7 +78,7 @@ static void file_close(void *v)
 	reftable_free(b);
 }
 
-static int file_read_block(void *v, struct reftable_block *dest, uint64_t off,
+static ssize_t file_read_block(void *v, struct reftable_block *dest, uint64_t off,
 			       uint32_t size)
 {
 	struct file_block_source *b = v;

reftable/reader.c

@@ -20,11 +20,11 @@ uint64_t block_source_size(struct reftable_block_source *source)
 	return source->ops->size(source->arg);
 }
 
-int block_source_read_block(struct reftable_block_source *source,
+ssize_t block_source_read_block(struct reftable_block_source *source,
 				struct reftable_block *dest, uint64_t off,
 				uint32_t size)
 {
-	int result = source->ops->read_block(source->arg, dest, off, size);
+	ssize_t result = source->ops->read_block(source->arg, dest, off, size);
 	dest->source = *source;
 	return result;
 }
@@ -57,14 +57,17 @@ static int reader_get_block(struct reftable_reader *r,
 			    struct reftable_block *dest, uint64_t off,
 			    uint32_t sz)
 {
+	ssize_t bytes_read;
 	if (off >= r->size)
 		return 0;
-
+	if (off + sz > r->size)
-	if (off + sz > r->size) {
 		sz = r->size - off;
-	}
 
-	return block_source_read_block(&r->source, dest, off, sz);
+	bytes_read = block_source_read_block(&r->source, dest, off, sz);
+	if (bytes_read < 0)
+		return (int)bytes_read;
+
+	return 0;
 }
 
 enum reftable_hash reftable_reader_hash_id(struct reftable_reader *r)
@@ -601,6 +604,7 @@ int reftable_reader_new(struct reftable_reader **out,
 	struct reftable_reader *r;
 	uint64_t file_size = block_source_size(source);
 	uint32_t read_size;
+	ssize_t bytes_read;
 	int err;
 
 	REFTABLE_CALLOC_ARRAY(r, 1);
@@ -619,8 +623,8 @@ int reftable_reader_new(struct reftable_reader **out,
 		goto done;
 	}
 
-	err = block_source_read_block(source, &header, 0, read_size);
+	bytes_read = block_source_read_block(source, &header, 0, read_size);
-	if (err != read_size) {
+	if (bytes_read < 0 || (size_t)bytes_read != read_size) {
 		err = REFTABLE_IO_ERROR;
 		goto done;
 	}
@@ -645,9 +649,9 @@ int reftable_reader_new(struct reftable_reader **out,
 	r->hash_id = 0;
 	r->refcount = 1;
 
-	err = block_source_read_block(source, &footer, r->size,
+	bytes_read = block_source_read_block(source, &footer, r->size,
 					     footer_size(r->version));
-	if (err != footer_size(r->version)) {
+	if (bytes_read < 0 || (size_t)bytes_read != footer_size(r->version)) {
 		err = REFTABLE_IO_ERROR;
 		goto done;
 	}
@@ -750,7 +754,7 @@ static int reftable_reader_refs_for_unindexed(struct reftable_reader *r,
 	struct table_iter *ti;
 	struct filtering_ref_iterator *filter = NULL;
 	struct filtering_ref_iterator empty = FILTERING_REF_ITERATOR_INIT;
-	int oid_len = hash_size(r->hash_id);
+	uint32_t oid_len = hash_size(r->hash_id);
 	int err;
 
 	REFTABLE_ALLOC_ARRAY(ti, 1);

reftable/reader.h

@@ -16,7 +16,7 @@ license that can be found in the LICENSE file or at
 
 uint64_t block_source_size(struct reftable_block_source *source);
 
-int block_source_read_block(struct reftable_block_source *source,
+ssize_t block_source_read_block(struct reftable_block_source *source,
 				struct reftable_block *dest, uint64_t off,
 				uint32_t size);
 void block_source_close(struct reftable_block_source *source);

reftable/record.c

@@ -21,47 +21,49 @@ static void *reftable_record_data(struct reftable_record *rec);
 
 int get_var_int(uint64_t *dest, struct string_view *in)
 {
-	int ptr = 0;
+	const unsigned char *buf = in->buf;
+	unsigned char c;
 	uint64_t val;
 
-	if (in->len == 0)
+	if (!in->len)
 		return -1;
-	val = in->buf[ptr] & 0x7f;
+	c = *buf++;
+	val = c & 0x7f;
 
-	while (in->buf[ptr] & 0x80) {
+	while (c & 0x80) {
-		ptr++;
+		/*
-		if (ptr > in->len) {
+		 * We use a micro-optimization here: whenever we see that the
+		 * 0x80 bit is set, we know that the remainder of the value
+		 * cannot be 0. The zero-values thus doesn't need to be encoded
+		 * at all, which is why we subtract 1 when encoding and add 1
+		 * when decoding.
+		 *
+		 * This allows us to save a byte in some edge cases.
+		 */
+		val += 1;
+		if (!val || (val & (uint64_t)(~0ULL << (64 - 7))))
+			return -1; /* overflow */
+		if (buf >= in->buf + in->len)
 			return -1;
-		}
+		c = *buf++;
-		val = (val + 1) << 7 | (uint64_t)(in->buf[ptr] & 0x7f);
+		val = (val << 7) + (c & 0x7f);
 	}
 
 	*dest = val;
-	return ptr + 1;
+	return buf - in->buf;
 }
 
-int put_var_int(struct string_view *dest, uint64_t val)
+int put_var_int(struct string_view *dest, uint64_t value)
 {
-	uint8_t buf[10] = { 0 };
+	unsigned char varint[10];
-	int i = 9;
+	unsigned pos = sizeof(varint) - 1;
-	int n = 0;
+	varint[pos] = value & 0x7f;
-	buf[i] = (uint8_t)(val & 0x7f);
+	while (value >>= 7)
-	i--;
+		varint[--pos] = 0x80 | (--value & 0x7f);
-	while (1) {
+	if (dest->len < sizeof(varint) - pos)
-		val >>= 7;
-		if (!val) {
-			break;
-		}
-		val--;
-		buf[i] = 0x80 | (uint8_t)(val & 0x7f);
-		i--;
-	}
-
-	n = sizeof(buf) - i - 1;
-	if (dest->len < n)
 		return -1;
-	memcpy(dest->buf, &buf[i + 1], n);
+	memcpy(dest->buf, varint + pos, sizeof(varint) - pos);
-	return n;
+	return sizeof(varint) - pos;
 }
 
 int reftable_is_block_type(uint8_t typ)
@@ -124,7 +126,7 @@ static int decode_string(struct reftable_buf *dest, struct string_view in)
 static int encode_string(const char *str, struct string_view s)
 {
 	struct string_view start = s;
-	int l = strlen(str);
+	size_t l = strlen(str);
 	int n = put_var_int(&s, l);
 	if (n < 0)
 		return -1;
@@ -142,9 +144,9 @@ int reftable_encode_key(int *restart, struct string_view dest,
 			uint8_t extra)
 {
 	struct string_view start = dest;
-	int prefix_len = common_prefix_size(&prev_key, &key);
+	size_t prefix_len = common_prefix_size(&prev_key, &key);
 	uint64_t suffix_len = key.len - prefix_len;
-	int n = put_var_int(&dest, (uint64_t)prefix_len);
+	int n = put_var_int(&dest, prefix_len);
 	if (n < 0)
 		return -1;
 	string_view_consume(&dest, n);
@@ -227,7 +229,7 @@ static int reftable_ref_record_key(const void *r, struct reftable_buf *dest)
 }
 
 static int reftable_ref_record_copy_from(void *rec, const void *src_rec,
-					 int hash_size)
+					 uint32_t hash_size)
 {
 	struct reftable_ref_record *ref = rec;
 	const struct reftable_ref_record *src = src_rec;
@@ -235,8 +237,6 @@ static int reftable_ref_record_copy_from(void *rec, const void *src_rec,
 	size_t refname_cap = 0;
 	int err;
 
-	assert(hash_size > 0);
-
 	SWAP(refname, ref->refname);
 	SWAP(refname_cap, ref->refname_cap);
 	reftable_ref_record_release(ref);
@@ -317,13 +317,12 @@ static uint8_t reftable_ref_record_val_type(const void *rec)
 }
 
 static int reftable_ref_record_encode(const void *rec, struct string_view s,
-				      int hash_size)
+				      uint32_t hash_size)
 {
 	const struct reftable_ref_record *r =
 		(const struct reftable_ref_record *)rec;
 	struct string_view start = s;
 	int n = put_var_int(&s, r->update_index);
-	assert(hash_size > 0);
 	if (n < 0)
 		return -1;
 	string_view_consume(&s, n);
@@ -363,7 +362,7 @@ static int reftable_ref_record_encode(const void *rec, struct string_view s,
 
 static int reftable_ref_record_decode(void *rec, struct reftable_buf key,
 				      uint8_t val_type, struct string_view in,
-				      int hash_size, struct reftable_buf *scratch)
+				      uint32_t hash_size, struct reftable_buf *scratch)
 {
 	struct reftable_ref_record *r = rec;
 	struct string_view start = in;
@@ -372,8 +371,6 @@ static int reftable_ref_record_decode(void *rec, struct reftable_buf key,
 	size_t refname_cap = 0;
 	int n, err;
 
-	assert(hash_size > 0);
-
 	n = get_var_int(&update_index, &in);
 	if (n < 0)
 		return n;
@@ -449,7 +446,7 @@ static int reftable_ref_record_is_deletion_void(const void *p)
 }
 
 static int reftable_ref_record_equal_void(const void *a,
-					  const void *b, int hash_size)
+					  const void *b, uint32_t hash_size)
 {
 	struct reftable_ref_record *ra = (struct reftable_ref_record *) a;
 	struct reftable_ref_record *rb = (struct reftable_ref_record *) b;
@@ -493,7 +490,7 @@ static void reftable_obj_record_release(void *rec)
 }
 
 static int reftable_obj_record_copy_from(void *rec, const void *src_rec,
-					 int hash_size UNUSED)
+					 uint32_t hash_size UNUSED)
 {
 	struct reftable_obj_record *obj = rec;
 	const struct reftable_obj_record *src = src_rec;
@@ -525,7 +522,7 @@ static uint8_t reftable_obj_record_val_type(const void *rec)
 }
 
 static int reftable_obj_record_encode(const void *rec, struct string_view s,
-				      int hash_size UNUSED)
+				      uint32_t hash_size UNUSED)
 {
 	const struct reftable_obj_record *r = rec;
 	struct string_view start = s;
@@ -560,7 +557,7 @@ static int reftable_obj_record_encode(const void *rec, struct string_view s,
 
 static int reftable_obj_record_decode(void *rec, struct reftable_buf key,
 				      uint8_t val_type, struct string_view in,
-				      int hash_size UNUSED,
+				      uint32_t hash_size UNUSED,
 				      struct reftable_buf *scratch UNUSED)
 {
 	struct string_view start = in;
@@ -568,7 +565,6 @@ static int reftable_obj_record_decode(void *rec, struct reftable_buf key,
 	uint64_t count = val_type;
 	int n = 0;
 	uint64_t last;
-	int j;
 
 	reftable_obj_record_release(r);
 
@@ -603,8 +599,7 @@ static int reftable_obj_record_decode(void *rec, struct reftable_buf key,
 	string_view_consume(&in, n);
 
 	last = r->offsets[0];
-	j = 1;
+	for (uint64_t j = 1; j < count; j++) {
-	while (j < count) {
 		uint64_t delta = 0;
 		int n = get_var_int(&delta, &in);
 		if (n < 0) {
@@ -613,7 +608,6 @@ static int reftable_obj_record_decode(void *rec, struct reftable_buf key,
 		string_view_consume(&in, n);
 
 		last = r->offsets[j] = (delta + last);
-		j++;
 	}
 	return start.len - in.len;
 }
@@ -624,7 +618,7 @@ static int not_a_deletion(const void *p UNUSED)
 }
 
 static int reftable_obj_record_equal_void(const void *a, const void *b,
-					  int hash_size UNUSED)
+					  uint32_t hash_size UNUSED)
 {
 	struct reftable_obj_record *ra = (struct reftable_obj_record *) a;
 	struct reftable_obj_record *rb = (struct reftable_obj_record *) b;
@@ -699,7 +693,7 @@ static int reftable_log_record_key(const void *r, struct reftable_buf *dest)
 }
 
 static int reftable_log_record_copy_from(void *rec, const void *src_rec,
-					 int hash_size)
+					 uint32_t hash_size)
 {
 	struct reftable_log_record *dst = rec;
 	const struct reftable_log_record *src =
@@ -780,7 +774,7 @@ static uint8_t reftable_log_record_val_type(const void *rec)
 }
 
 static int reftable_log_record_encode(const void *rec, struct string_view s,
-				      int hash_size)
+				      uint32_t hash_size)
 {
 	const struct reftable_log_record *r = rec;
 	struct string_view start = s;
@@ -828,7 +822,7 @@ static int reftable_log_record_encode(const void *rec, struct string_view s,
 
 static int reftable_log_record_decode(void *rec, struct reftable_buf key,
 				      uint8_t val_type, struct string_view in,
-				      int hash_size, struct reftable_buf *scratch)
+				      uint32_t hash_size, struct reftable_buf *scratch)
 {
 	struct string_view start = in;
 	struct reftable_log_record *r = rec;
@@ -976,7 +970,7 @@ static int null_streq(const char *a, const char *b)
 }
 
 static int reftable_log_record_equal_void(const void *a,
-					  const void *b, int hash_size)
+					  const void *b, uint32_t hash_size)
 {
 	return reftable_log_record_equal((struct reftable_log_record *) a,
 					 (struct reftable_log_record *) b,
@@ -1000,7 +994,7 @@ static int reftable_log_record_cmp_void(const void *_a, const void *_b)
 }
 
 int reftable_log_record_equal(const struct reftable_log_record *a,
-			      const struct reftable_log_record *b, int hash_size)
+			      const struct reftable_log_record *b, uint32_t hash_size)
 {
 	if (!(null_streq(a->refname, b->refname) &&
 	      a->update_index == b->update_index &&
@@ -1054,7 +1048,7 @@ static int reftable_index_record_key(const void *r, struct reftable_buf *dest)
 }
 
 static int reftable_index_record_copy_from(void *rec, const void *src_rec,
-					   int hash_size UNUSED)
+					   uint32_t hash_size UNUSED)
 {
 	struct reftable_index_record *dst = rec;
 	const struct reftable_index_record *src = src_rec;
@@ -1081,7 +1075,7 @@ static uint8_t reftable_index_record_val_type(const void *rec UNUSED)
 }
 
 static int reftable_index_record_encode(const void *rec, struct string_view out,
-					int hash_size UNUSED)
+					uint32_t hash_size UNUSED)
 {
 	const struct reftable_index_record *r =
 		(const struct reftable_index_record *)rec;
@@ -1099,7 +1093,7 @@ static int reftable_index_record_encode(const void *rec, struct string_view out,
 static int reftable_index_record_decode(void *rec, struct reftable_buf key,
 					uint8_t val_type UNUSED,
 					struct string_view in,
-					int hash_size UNUSED,
+					uint32_t hash_size UNUSED,
 					struct reftable_buf *scratch UNUSED)
 {
 	struct string_view start = in;
@@ -1120,7 +1114,7 @@ static int reftable_index_record_decode(void *rec, struct reftable_buf key,
 }
 
 static int reftable_index_record_equal(const void *a, const void *b,
-				       int hash_size UNUSED)
+				       uint32_t hash_size UNUSED)
 {
 	struct reftable_index_record *ia = (struct reftable_index_record *) a;
 	struct reftable_index_record *ib = (struct reftable_index_record *) b;
@@ -1154,14 +1148,14 @@ int reftable_record_key(struct reftable_record *rec, struct reftable_buf *dest)
 }
 
 int reftable_record_encode(struct reftable_record *rec, struct string_view dest,
-			   int hash_size)
+			   uint32_t hash_size)
 {
 	return reftable_record_vtable(rec)->encode(reftable_record_data(rec),
 						   dest, hash_size);
 }
 
 int reftable_record_copy_from(struct reftable_record *rec,
-			       struct reftable_record *src, int hash_size)
+			       struct reftable_record *src, uint32_t hash_size)
 {
 	assert(src->type == rec->type);
 
@@ -1176,7 +1170,7 @@ uint8_t reftable_record_val_type(struct reftable_record *rec)
 }
 
 int reftable_record_decode(struct reftable_record *rec, struct reftable_buf key,
-			   uint8_t extra, struct string_view src, int hash_size,
+			   uint8_t extra, struct string_view src, uint32_t hash_size,
 			   struct reftable_buf *scratch)
 {
 	return reftable_record_vtable(rec)->decode(reftable_record_data(rec),
@@ -1203,7 +1197,7 @@ int reftable_record_cmp(struct reftable_record *a, struct reftable_record *b)
 		reftable_record_data(a), reftable_record_data(b));
 }
 
-int reftable_record_equal(struct reftable_record *a, struct reftable_record *b, int hash_size)
+int reftable_record_equal(struct reftable_record *a, struct reftable_record *b, uint32_t hash_size)
 {
 	if (a->type != b->type)
 		return 0;
@@ -1211,7 +1205,7 @@ int reftable_record_equal(struct reftable_record *a, struct reftable_record *b,
 		reftable_record_data(a), reftable_record_data(b), hash_size);
 }
 
-static int hash_equal(const unsigned char *a, const unsigned char *b, int hash_size)
+static int hash_equal(const unsigned char *a, const unsigned char *b, uint32_t hash_size)
 {
 	if (a && b)
 		return !memcmp(a, b, hash_size);
@@ -1220,9 +1214,8 @@ static int hash_equal(const unsigned char *a, const unsigned char *b, int hash_s
 }
 
 int reftable_ref_record_equal(const struct reftable_ref_record *a,
-			      const struct reftable_ref_record *b, int hash_size)
+			      const struct reftable_ref_record *b, uint32_t hash_size)
 {
-	assert(hash_size > 0);
 	if (!null_streq(a->refname, b->refname))
 		return 0;
 

reftable/record.h

@@ -32,8 +32,10 @@ static inline void string_view_consume(struct string_view *s, int n)
 	s->len -= n;
 }
 
-/* utilities for de/encoding varints */
+/*
-
+ * Decode and encode a varint. Returns the number of bytes read/written, or a
+ * negative value in case encoding/decoding the varint has failed.
+ */
 int get_var_int(uint64_t *dest, struct string_view *in);
 int put_var_int(struct string_view *dest, uint64_t val);
 
@@ -45,18 +47,18 @@ struct reftable_record_vtable {
 	/* The record type of ('r' for ref). */
 	uint8_t type;
 
-	int (*copy_from)(void *dest, const void *src, int hash_size);
+	int (*copy_from)(void *dest, const void *src, uint32_t hash_size);
 
 	/* a value of [0..7], indicating record subvariants (eg. ref vs. symref
 	 * vs ref deletion) */
 	uint8_t (*val_type)(const void *rec);
 
 	/* encodes rec into dest, returning how much space was used. */
-	int (*encode)(const void *rec, struct string_view dest, int hash_size);
+	int (*encode)(const void *rec, struct string_view dest, uint32_t hash_size);
 
 	/* decode data from `src` into the record. */
 	int (*decode)(void *rec, struct reftable_buf key, uint8_t extra,
-		      struct string_view src, int hash_size,
+		      struct string_view src, uint32_t hash_size,
 		      struct reftable_buf *scratch);
 
 	/* deallocate and null the record. */
@@ -66,16 +68,13 @@ struct reftable_record_vtable {
 	int (*is_deletion)(const void *rec);
 
 	/* Are two records equal? This assumes they have the same type. Returns 0 for non-equal. */
-	int (*equal)(const void *a, const void *b, int hash_size);
+	int (*equal)(const void *a, const void *b, uint32_t hash_size);
 
 	/*
 	 * Compare keys of two records with each other. The records must have
 	 * the same type.
 	 */
 	int (*cmp)(const void *a, const void *b);
-
-	/* Print on stdout, for debugging. */
-	void (*print)(const void *rec, int hash_size);
 };
 
 /* returns true for recognized block types. Block start with the block type. */
@@ -136,16 +135,16 @@ 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);
+int reftable_record_equal(struct reftable_record *a, struct reftable_record *b, uint32_t hash_size);
 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);
+			      struct reftable_record *src, uint32_t 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);
+			   uint32_t hash_size);
 int reftable_record_decode(struct reftable_record *rec, struct reftable_buf key,
 			   uint8_t extra, struct string_view src,
-			   int hash_size, struct reftable_buf *scratch);
+			   uint32_t 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/reftable-blocksource.h

@@ -22,7 +22,7 @@ struct reftable_block_source {
  * so it can return itself into the pool. */
 struct reftable_block {
 	uint8_t *data;
-	int len;
+	size_t len;
 	struct reftable_block_source source;
 };
 
@@ -31,10 +31,13 @@ struct reftable_block_source_vtable {
 	/* returns the size of a block source */
 	uint64_t (*size)(void *source);
 
-	/* reads a segment from the block source. It is an error to read
+	/*
-	   beyond the end of the block */
+	 * Reads a segment from the block source. It is an error to read beyond
-	int (*read_block)(void *source, struct reftable_block *dest,
+	 * the end of the block.
+	 */
+	ssize_t (*read_block)(void *source, struct reftable_block *dest,
 			     uint64_t off, uint32_t size);
+
 	/* mark the block as read; may return the data back to malloc */
 	void (*return_block)(void *source, struct reftable_block *blockp);
 

reftable/reftable-record.h

@@ -65,7 +65,7 @@ void reftable_ref_record_release(struct reftable_ref_record *ref);
 
 /* returns whether two reftable_ref_records are the same. Useful for testing. */
 int reftable_ref_record_equal(const struct reftable_ref_record *a,
-			      const struct reftable_ref_record *b, int hash_size);
+			      const struct reftable_ref_record *b, uint32_t hash_size);
 
 /* reftable_log_record holds a reflog entry */
 struct reftable_log_record {
@@ -105,6 +105,6 @@ void reftable_log_record_release(struct reftable_log_record *log);
 
 /* returns whether two records are equal. Useful for testing. */
 int reftable_log_record_equal(const struct reftable_log_record *a,
-			      const struct reftable_log_record *b, int hash_size);
+			      const struct reftable_log_record *b, uint32_t hash_size);
 
 #endif

reftable/reftable-writer.h

@@ -84,7 +84,7 @@ struct reftable_block_stats {
 	/* total number of entries written */
 	int entries;
 	/* total number of key restarts */
-	int restarts;
+	uint32_t restarts;
 	/* total number of blocks */
 	int blocks;
 	/* total number of index blocks */

reftable/stack.c

@@ -220,9 +220,9 @@ void reftable_stack_destroy(struct reftable_stack *st)
 	}
 
 	if (st->readers) {
-		int i = 0;
 		struct reftable_buf filename = REFTABLE_BUF_INIT;
-		for (i = 0; i < st->readers_len; i++) {
+
+		for (size_t i = 0; i < st->readers_len; i++) {
 			const char *name = reader_name(st->readers[i]);
 			int try_unlinking = 1;
 
@@ -238,6 +238,7 @@ void reftable_stack_destroy(struct reftable_stack *st)
 				unlink(filename.buf);
 			}
 		}
+
 		reftable_buf_release(&filename);
 		st->readers_len = 0;
 		REFTABLE_FREE_AND_NULL(st->readers);
@@ -568,7 +569,6 @@ static int stack_uptodate(struct reftable_stack *st)
 {
 	char **names = NULL;
 	int err;
-	int i = 0;
 
 	/*
 	 * When we have cached stat information available then we use it to
@@ -608,7 +608,7 @@ static int stack_uptodate(struct reftable_stack *st)
 	if (err < 0)
 		return err;
 
-	for (i = 0; i < st->readers_len; i++) {
+	for (size_t i = 0; i < st->readers_len; i++) {
 		if (!names[i]) {
 			err = 1;
 			goto done;
@@ -1767,14 +1767,12 @@ static int reftable_stack_clean_locked(struct reftable_stack *st)
 	}
 
 	while ((d = readdir(dir))) {
-		int i = 0;
 		int found = 0;
 		if (!is_table_name(d->d_name))
 			continue;
 
-		for (i = 0; !found && i < st->readers_len; i++) {
+		for (size_t i = 0; !found && i < st->readers_len; i++)
 			found = !strcmp(reader_name(st->readers[i]), d->d_name);
-		}
 		if (found)
 			continue;
 

reftable/system.h

@@ -11,8 +11,6 @@ license that can be found in the LICENSE file or at
 
 /* This header glues the reftable library to the rest of Git */
 
-#define DISABLE_SIGN_COMPARE_WARNINGS
-
 #include "git-compat-util.h"
 
 /*

reftable/writer.c

@@ -577,7 +577,7 @@ static int writer_finish_section(struct reftable_writer *w)
 
 struct common_prefix_arg {
 	struct reftable_buf *last;
-	int max;
+	size_t max;
 };
 
 static void update_common(void *void_arg, void *key)
@@ -585,11 +585,10 @@ static void update_common(void *void_arg, void *key)
 	struct common_prefix_arg *arg = void_arg;
 	struct obj_index_tree_node *entry = key;
 	if (arg->last) {
-		int n = common_prefix_size(&entry->hash, arg->last);
+		size_t n = common_prefix_size(&entry->hash, arg->last);
-		if (n > arg->max) {
+		if (n > arg->max)
 			arg->max = n;
 	}
-	}
 	arg->last = &entry->hash;
 }
 

t/unit-tests/t-reftable-basics.c

@@ -120,7 +120,7 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED)
 		for (size_t i = 0; i < ARRAY_SIZE(cases); i++) {
 			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);
+			check_uint(common_prefix_size(&a, &b), ==, cases[i].want);
 			reftable_buf_reset(&a);
 			reftable_buf_reset(&b);
 		}

t/unit-tests/t-reftable-readwrite.c

@@ -643,7 +643,7 @@ static void t_write_empty_table(void)
 	check_int(err, ==, REFTABLE_EMPTY_TABLE_ERROR);
 	reftable_writer_free(w);
 
-	check_int(buf.len, ==, header_size(1) + footer_size(1));
+	check_uint(buf.len, ==, header_size(1) + footer_size(1));
 
 	block_source_from_buf(&source, &buf);
 

t/unit-tests/t-reftable-record.c

@@ -58,9 +58,25 @@ static void t_varint_roundtrip(void)
 	}
 }
 
+static void t_varint_overflow(void)
+{
+	unsigned char buf[] = {
+		0xFF, 0xFF, 0xFF, 0xFF,
+		0xFF, 0xFF, 0xFF, 0xFF,
+		0xFF, 0x00,
+	};
+	struct string_view view = {
+		.buf = buf,
+		.len = sizeof(buf),
+	};
+	uint64_t value;
+	int err = get_var_int(&value, &view);
+	check_int(err, ==, -1);
+}
+
 static void set_hash(uint8_t *h, int j)
 {
-	for (int i = 0; i < hash_size(REFTABLE_HASH_SHA1); i++)
+	for (size_t i = 0; i < hash_size(REFTABLE_HASH_SHA1); i++)
 		h[i] = (j >> i) & 0xff;
 }
 
@@ -544,6 +560,7 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED)
 	TEST(t_reftable_log_record_roundtrip(), "record operations work on log record");
 	TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record");
 	TEST(t_varint_roundtrip(), "put_var_int and get_var_int work");
+	TEST(t_varint_overflow(), "get_var_int notices an integer overflow");
 	TEST(t_key_roundtrip(), "reftable_encode_key and reftable_decode_key work");
 	TEST(t_reftable_obj_record_roundtrip(), "record operations work on obj record");
 	TEST(t_reftable_index_record_roundtrip(), "record operations work on index record");