Merge branch 'ps/reftable-fixes' into next

Bunch of small fix-ups to the reftable code.

* ps/reftable-fixes:
  reftable/block: reuse buffer to compute record keys
  reftable/block: introduce macro to initialize `struct block_iter`
  reftable/merged: reuse buffer to compute record keys
  reftable/stack: fix use of unseeded randomness
  reftable/stack: fix stale lock when dying
  reftable/stack: reuse buffers when reloading stack
  reftable/stack: perform auto-compaction with transactional interface
  reftable/stack: verify that `reftable_stack_add()` uses auto-compaction
  reftable: handle interrupted writes
  reftable: handle interrupted reads
  reftable: wrap EXPECT macros in do/while

reftable/block.c

@@ -323,30 +323,28 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec)
  .len = it->br->block_len - it->next_off,
  };
  struct string_view start = in;
- struct strbuf key = STRBUF_INIT;
  uint8_t extra = 0;
  int n = 0;
 
  if (it->next_off >= it->br->block_len)
  return 1;
 
- n = reftable_decode_key(&key, &extra, it->last_key, in);
+ n = reftable_decode_key(&it->key, &extra, it->last_key, in);
  if (n < 0)
  return -1;
 
- if (!key.len)
+ if (!it->key.len)
  return REFTABLE_FORMAT_ERROR;
 
  string_view_consume(&in, n);
- n = reftable_record_decode(rec, key, extra, in, it->br->hash_size);
+ n = reftable_record_decode(rec, it->key, extra, in, it->br->hash_size);
  if (n < 0)
  return -1;
  string_view_consume(&in, n);
 
  strbuf_reset(&it->last_key);
- strbuf_addbuf(&it->last_key, &key);
+ strbuf_addbuf(&it->last_key, &it->key);
  it->next_off += start.len - in.len;
- strbuf_release(&key);
  return 0;
 }
 
@@ -377,6 +375,7 @@ int block_iter_seek(struct block_iter *it, struct strbuf *want)
 void block_iter_close(struct block_iter *it)
 {
  strbuf_release(&it->last_key);
+ strbuf_release(&it->key);
 }
 
 int block_reader_seek(struct block_reader *br, struct block_iter *it,
@@ -387,11 +386,8 @@ int block_reader_seek(struct block_reader *br, struct block_iter *it,
  .r = br,
  };
  struct reftable_record rec = reftable_new_record(block_reader_type(br));
- struct strbuf key = STRBUF_INIT;
  int err = 0;
- struct block_iter next = {
- .last_key = STRBUF_INIT,
- };
+ struct block_iter next = BLOCK_ITER_INIT;
 
  int i = binsearch(br->restart_count, &restart_key_less, &args);
  if (args.error) {
@@ -416,8 +412,8 @@ int block_reader_seek(struct block_reader *br, struct block_iter *it,
  if (err < 0)
  goto done;
 
- reftable_record_key(&rec, &key);
- if (err > 0 || strbuf_cmp(&key, want) >= 0) {
+ reftable_record_key(&rec, &it->key);
+ if (err > 0 || strbuf_cmp(&it->key, want) >= 0) {
  err = 0;
  goto done;
  }
@@ -426,8 +422,7 @@ int block_reader_seek(struct block_reader *br, struct block_iter *it,
  }
 
 done:
- strbuf_release(&key);
- strbuf_release(&next.last_key);
+ block_iter_close(&next);
  reftable_record_release(&rec);
 
  return err;

reftable/block.h

@@ -84,8 +84,14 @@ struct block_iter {
 
  /* key for last entry we read. */
  struct strbuf last_key;
+ struct strbuf key;
 };
 
+#define BLOCK_ITER_INIT { \
+ .last_key = STRBUF_INIT, \
+ .key = STRBUF_INIT, \
+}
+
 /* 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,

reftable/block_test.c

@@ -32,7 +32,7 @@ static void test_block_read_write(void)
  int i = 0;
  int n;
  struct block_reader br = { 0 };
- struct block_iter it = { .last_key = STRBUF_INIT };
+ struct block_iter it = BLOCK_ITER_INIT;
  int j = 0;
  struct strbuf want = STRBUF_INIT;
 
@@ -87,7 +87,7 @@ static void test_block_read_write(void)
  block_iter_close(&it);
 
  for (i = 0; i < N; i++) {
- struct block_iter it = { .last_key = STRBUF_INIT };
+ struct block_iter it = BLOCK_ITER_INIT;
  strbuf_reset(&want);
  strbuf_addstr(&want, names[i]);
 

reftable/blocksource.c

@@ -109,7 +109,7 @@ static int file_read_block(void *v, struct reftable_block *dest, uint64_t off,
  struct file_block_source *b = v;
  assert(off + size <= b->size);
  dest->data = reftable_malloc(size);
- if (pread(b->fd, dest->data, size, off) != size)
+ if (pread_in_full(b->fd, dest->data, size, off) != size)
  return -1;
  dest->len = size;
  return size;

reftable/iter.h

@@ -53,10 +53,10 @@ struct indexed_table_ref_iter {
  int is_finished;
 };
 
-#define INDEXED_TABLE_REF_ITER_INIT   \
- {  \
-  .cur = { .last_key = STRBUF_INIT }, .oid = STRBUF_INIT, \
- }
+#define INDEXED_TABLE_REF_ITER_INIT { \
+ .cur = BLOCK_ITER_INIT, \
+ .oid = STRBUF_INIT, \
+}
 
 void iterator_from_indexed_table_ref_iter(struct reftable_iterator *it,
  struct indexed_table_ref_iter *itr);

reftable/merged.c

@@ -52,6 +52,8 @@ static void merged_iter_close(void *p)
  reftable_iterator_destroy(&mi->stack[i]);
  }
  reftable_free(mi->stack);
+ strbuf_release(&mi->key);
+ strbuf_release(&mi->entry_key);
 }
 
 static int merged_iter_advance_nonnull_subiter(struct merged_iter *mi,
@@ -85,7 +87,6 @@ static int merged_iter_advance_subiter(struct merged_iter *mi, size_t idx)
 static int merged_iter_next_entry(struct merged_iter *mi,
  struct reftable_record *rec)
 {
- struct strbuf entry_key = STRBUF_INIT;
  struct pq_entry entry = { 0 };
  int err = 0;
 
@@ -105,33 +106,31 @@ static int merged_iter_next_entry(struct merged_iter *mi,
  such a deployment, the loop below must be changed to collect all
  entries for the same key, and return new the newest one.
  */
- reftable_record_key(&entry.rec, &entry_key);
+ reftable_record_key(&entry.rec, &mi->entry_key);
  while (!merged_iter_pqueue_is_empty(mi->pq)) {
  struct pq_entry top = merged_iter_pqueue_top(mi->pq);
- struct strbuf k = STRBUF_INIT;
- int err = 0, cmp = 0;
+ int cmp = 0;
 
- reftable_record_key(&top.rec, &k);
+ reftable_record_key(&top.rec, &mi->key);
 
- cmp = strbuf_cmp(&k, &entry_key);
- strbuf_release(&k);
-
- if (cmp > 0) {
+ cmp = strbuf_cmp(&mi->key, &mi->entry_key);
+ if (cmp > 0)
  break;
- }
 
  merged_iter_pqueue_remove(&mi->pq);
  err = merged_iter_advance_subiter(mi, top.index);
- if (err < 0) {
- return err;
- }
+ if (err < 0)
+ goto done;
  reftable_record_release(&top.rec);
  }
 
  reftable_record_copy_from(rec, &entry.rec, hash_size(mi->hash_id));
+
+done:
  reftable_record_release(&entry.rec);
- strbuf_release(&entry_key);
- return 0;
+ strbuf_release(&mi->entry_key);
+ strbuf_release(&mi->key);
+ return err;
 }
 
 static int merged_iter_next(struct merged_iter *mi, struct reftable_record *rec)
@@ -248,6 +247,8 @@ static int merged_table_seek_record(struct reftable_merged_table *mt,
  .typ = reftable_record_type(rec),
  .hash_id = mt->hash_id,
  .suppress_deletions = mt->suppress_deletions,
+ .key = STRBUF_INIT,
+ .entry_key = STRBUF_INIT,
  };
  int n = 0;
  int err = 0;

reftable/merged.h

@@ -31,6 +31,8 @@ struct merged_iter {
  uint8_t typ;
  int suppress_deletions;
  struct merged_iter_pqueue pq;
+ struct strbuf key;
+ struct strbuf entry_key;
 };
 
 void merged_table_release(struct reftable_merged_table *mt);

reftable/reader.c

@@ -224,10 +224,9 @@ struct table_iter {
  struct block_iter bi;
  int is_finished;
 };
-#define TABLE_ITER_INIT \
- { \
- .bi = {.last_key = STRBUF_INIT } \
- }
+#define TABLE_ITER_INIT { \
+ .bi = BLOCK_ITER_INIT \
+}
 
 static void table_iter_copy_from(struct table_iter *dest,
  struct table_iter *src)

reftable/readwrite_test.c

@@ -141,8 +141,8 @@ static void test_log_buffer_size(void)
  */
  uint8_t hash1[GIT_SHA1_RAWSZ], hash2[GIT_SHA1_RAWSZ];
  for (i = 0; i < GIT_SHA1_RAWSZ; i++) {
- hash1[i] = (uint8_t)(rand() % 256);
- hash2[i] = (uint8_t)(rand() % 256);
+ hash1[i] = (uint8_t)(git_rand() % 256);
+ hash2[i] = (uint8_t)(git_rand() % 256);
  }
  log.value.update.old_hash = hash1;
  log.value.update.new_hash = hash2;
@@ -320,7 +320,7 @@ static void test_log_zlib_corruption(void)
  };
 
  for (i = 0; i < sizeof(message) - 1; i++)
- message[i] = (uint8_t)(rand() % 64 + ' ');
+ message[i] = (uint8_t)(git_rand() % 64 + ' ');
 
  reftable_writer_set_limits(w, 1, 1);