Merge branch 'hn/reftable-fixes'

Assorted fixlets in reftable code.

* hn/reftable-fixes:
  reftable: support preset file mode for writing
  reftable: signal overflow
  reftable: fix typo in header

reftable/block.h

@@ -21,7 +21,7 @@ struct block_writer {
 	uint8_t *buf;
 	uint32_t block_size;
 
-	/* Offset ofof the global header. Nonzero in the first block only. */
+	/* Offset of the global header. Nonzero in the first block only. */
 	uint32_t header_off;
 
 	/* How often to restart keys. */

reftable/error.c

@@ -32,6 +32,8 @@ const char *reftable_error_str(int err)
 		return "wrote empty table";
 	case REFTABLE_REFNAME_ERROR:
 		return "invalid refname";
+	case REFTABLE_ENTRY_TOO_BIG_ERROR:
+		return "entry too large";
 	case -1:
 		return "general error";
 	default:

reftable/readwrite_test.c

@@ -155,6 +155,40 @@ static void test_log_buffer_size(void)
 	strbuf_release(&buf);
 }
 
+static void test_log_overflow(void)
+{
+	struct strbuf buf = STRBUF_INIT;
+	char msg[256] = { 0 };
+	struct reftable_write_options opts = {
+		.block_size = ARRAY_SIZE(msg),
+	};
+	int err;
+	struct reftable_log_record
+		log = { .refname = "refs/heads/master",
+			.update_index = 0xa,
+			.value_type = REFTABLE_LOG_UPDATE,
+			.value = { .update = {
+					   .name = "Han-Wen Nienhuys",
+					   .email = "hanwen@google.com",
+					   .tz_offset = 100,
+					   .time = 0x5e430672,
+					   .message = msg,
+				   } } };
+	struct reftable_writer *w =
+		reftable_new_writer(&strbuf_add_void, &buf, &opts);
+
+	uint8_t hash1[GIT_SHA1_RAWSZ]  = {1}, hash2[GIT_SHA1_RAWSZ] = { 2 };
+
+	memset(msg, 'x', sizeof(msg) - 1);
+	log.value.update.old_hash = hash1;
+	log.value.update.new_hash = hash2;
+	reftable_writer_set_limits(w, update_index, update_index);
+	err = reftable_writer_add_log(w, &log);
+	EXPECT(err == REFTABLE_ENTRY_TOO_BIG_ERROR);
+	reftable_writer_free(w);
+	strbuf_release(&buf);
+}
+
 static void test_log_write_read(void)
 {
 	int N = 2;
@@ -648,5 +682,6 @@ int readwrite_test_main(int argc, const char *argv[])
 	RUN_TEST(test_table_refs_for_no_index);
 	RUN_TEST(test_table_refs_for_obj_index);
 	RUN_TEST(test_write_empty_table);
+	RUN_TEST(test_log_overflow);
 	return 0;
 }

reftable/reftable-error.h

@@ -53,6 +53,10 @@ enum reftable_error {
 
 	/* Invalid ref name. */
 	REFTABLE_REFNAME_ERROR = -10,
+
+	/* Entry does not fit. This can happen when writing outsize reflog
+	   messages. */
+	REFTABLE_ENTRY_TOO_BIG_ERROR = -11,
 };
 
 /* convert the numeric error code to a string. The string should not be

reftable/reftable-writer.h

@@ -35,6 +35,9 @@ struct reftable_write_options {
 	 */
 	uint32_t hash_id;
 
+	/* Default mode for creating files. If unset, use 0666 (+umask) */
+	unsigned int default_permissions;
+
 	/* boolean: do not check ref names for validity or dir/file conflicts.
 	 */
 	unsigned skip_name_check : 1;

reftable/stack.c

@@ -469,7 +469,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
 	strbuf_addstr(&add->lock_file_name, ".lock");
 
 	add->lock_file_fd = open(add->lock_file_name.buf,
-				 O_EXCL | O_CREAT | O_WRONLY, 0644);
+				 O_EXCL | O_CREAT | O_WRONLY, 0666);
 	if (add->lock_file_fd < 0) {
 		if (errno == EEXIST) {
 			err = REFTABLE_LOCK_ERROR;
@@ -478,6 +478,13 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
 		}
 		goto done;
 	}
+	if (st->config.default_permissions) {
+		if (chmod(add->lock_file_name.buf, st->config.default_permissions) < 0) {
+			err = REFTABLE_IO_ERROR;
+			goto done;
+		}
+	}
+
 	err = stack_uptodate(st);
 	if (err < 0)
 		goto done;
@@ -644,7 +651,12 @@ int reftable_addition_add(struct reftable_addition *add,
 		err = REFTABLE_IO_ERROR;
 		goto done;
 	}
-
+	if (add->stack->config.default_permissions) {
+		if (chmod(temp_tab_file_name.buf, add->stack->config.default_permissions)) {
+			err = REFTABLE_IO_ERROR;
+			goto done;
+		}
+	}
 	wr = reftable_new_writer(reftable_fd_write, &tab_fd,
 				 &add->stack->config);
 	err = write_table(wr, arg);
@@ -900,7 +912,7 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last,
 	strbuf_addstr(&lock_file_name, ".lock");
 
 	lock_file_fd =
-		open(lock_file_name.buf, O_EXCL | O_CREAT | O_WRONLY, 0644);
+		open(lock_file_name.buf, O_EXCL | O_CREAT | O_WRONLY, 0666);
 	if (lock_file_fd < 0) {
 		if (errno == EEXIST) {
 			err = 1;
@@ -931,8 +943,8 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last,
 		strbuf_addstr(&subtab_lock, ".lock");
 
 		sublock_file_fd = open(subtab_lock.buf,
-				       O_EXCL | O_CREAT | O_WRONLY, 0644);
-		if (sublock_file_fd > 0) {
+				       O_EXCL | O_CREAT | O_WRONLY, 0666);
+		if (sublock_file_fd >= 0) {
 			close(sublock_file_fd);
 		} else if (sublock_file_fd < 0) {
 			if (errno == EEXIST) {
@@ -967,7 +979,7 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last,
 		goto done;
 
 	lock_file_fd =
-		open(lock_file_name.buf, O_EXCL | O_CREAT | O_WRONLY, 0644);
+		open(lock_file_name.buf, O_EXCL | O_CREAT | O_WRONLY, 0666);
 	if (lock_file_fd < 0) {
 		if (errno == EEXIST) {
 			err = 1;
@@ -977,6 +989,12 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last,
 		goto done;
 	}
 	have_lock = 1;
+	if (st->config.default_permissions) {
+		if (chmod(lock_file_name.buf, st->config.default_permissions) < 0) {
+			err = REFTABLE_IO_ERROR;
+			goto done;
+		}
+	}
 
 	format_name(&new_table_name, st->readers[first]->min_update_index,
 		    st->readers[last]->max_update_index);

reftable/stack_test.c

@@ -17,6 +17,7 @@ license that can be found in the LICENSE file or at
 #include "record.h"
 #include "test_framework.h"
 #include "reftable-tests.h"
+#include "reader.h"
 
 #include <sys/types.h>
 #include <dirent.h>
@@ -138,8 +139,11 @@ static int write_test_log(struct reftable_writer *wr, void *arg)
 static void test_reftable_stack_add_one(void)
 {
 	char *dir = get_tmp_dir(__LINE__);
-
-	struct reftable_write_options cfg = { 0 };
+	struct strbuf scratch = STRBUF_INIT;
+	int mask = umask(002);
+	struct reftable_write_options cfg = {
+		.default_permissions = 0660,
+	};
 	struct reftable_stack *st = NULL;
 	int err;
 	struct reftable_ref_record ref = {
@@ -149,8 +153,7 @@ static void test_reftable_stack_add_one(void)
 		.value.symref = "master",
 	};
 	struct reftable_ref_record dest = { NULL };
-
-
+	struct stat stat_result = { 0 };
 	err = reftable_new_stack(&st, dir, cfg);
 	EXPECT_ERR(err);
 
@@ -160,6 +163,7 @@ static void test_reftable_stack_add_one(void)
 	err = reftable_stack_read_ref(st, ref.refname, &dest);
 	EXPECT_ERR(err);
 	EXPECT(0 == strcmp("master", dest.value.symref));
+	EXPECT(st->readers_len > 0);
 
 	printf("testing print functionality:\n");
 	err = reftable_stack_print_directory(dir, GIT_SHA1_FORMAT_ID);
@@ -168,9 +172,30 @@ static void test_reftable_stack_add_one(void)
 	err = reftable_stack_print_directory(dir, GIT_SHA256_FORMAT_ID);
 	EXPECT(err == REFTABLE_FORMAT_ERROR);
 
+#ifndef GIT_WINDOWS_NATIVE
+	strbuf_addstr(&scratch, dir);
+	strbuf_addstr(&scratch, "/tables.list");
+	err = stat(scratch.buf, &stat_result);
+	EXPECT(!err);
+	EXPECT((stat_result.st_mode & 0777) == cfg.default_permissions);
+
+	strbuf_reset(&scratch);
+	strbuf_addstr(&scratch, dir);
+	strbuf_addstr(&scratch, "/");
+	/* do not try at home; not an external API for reftable. */
+	strbuf_addstr(&scratch, st->readers[0]->name);
+	err = stat(scratch.buf, &stat_result);
+	EXPECT(!err);
+	EXPECT((stat_result.st_mode & 0777) == cfg.default_permissions);
+#else
+	(void) stat_result;
+#endif
+
 	reftable_ref_record_release(&dest);
 	reftable_stack_destroy(st);
+	strbuf_release(&scratch);
 	clear_dir(dir);
+	umask(mask);
 }
 
 static void test_reftable_stack_uptodate(void)

reftable/writer.c

@@ -239,6 +239,9 @@ static int writer_add_record(struct reftable_writer *w,
 	writer_reinit_block_writer(w, reftable_record_type(rec));
 	err = block_writer_add(w->block_writer, rec);
 	if (err < 0) {
+		/* we are writing into memory, so an error can only mean it
+		 * doesn't fit. */
+		err = REFTABLE_ENTRY_TOO_BIG_ERROR;
 		goto done;
 	}