Convert reflog to new errors

Cleaned up some other issues.

src/reflog.c

@@ -16,23 +16,21 @@ static int reflog_init(git_reflog **reflog, git_reference *ref)
 
 	*reflog = NULL;
 
-	log = git__malloc(sizeof(git_reflog));
-	if (log == NULL)
-		return GIT_ENOMEM;
-
-	memset(log, 0x0, sizeof(git_reflog));
+	log = git__calloc(1, sizeof(git_reflog));
+	GITERR_CHECK_ALLOC(log);
 
 	log->ref_name = git__strdup(ref->name);
+	GITERR_CHECK_ALLOC(log->ref_name);
 
 	if (git_vector_init(&log->entries, 0, NULL) < 0) {
 		git__free(log->ref_name);
 		git__free(log);
-		return GIT_ENOMEM;
+		return -1;
 	}
 
 	*reflog = log;
 
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static int reflog_write(const char *log_path, const char *oid_old,
@@ -42,9 +40,22 @@ static int reflog_write(const char *log_path, const char *oid_old,
 	int error;
 	git_buf log = GIT_BUF_INIT;
 	git_filebuf fbuf = GIT_FILEBUF_INIT;
+	bool trailing_newline = false;
 
 	assert(log_path && oid_old && oid_new && committer);
 
+	if (msg) {
+		const char *newline = strchr(msg, '\n');
+		if (newline) {
+			if (*(newline + 1) == '\0')
+				trailing_newline = true;
+			else {
+				giterr_set(GITERR_INVALID, "Reflog message cannot contain newline");
+				return -1;
+			}
+		}
+	}
+
 	git_buf_puts(&log, oid_old);
 	git_buf_putc(&log, ' ');
 
@@ -54,68 +65,58 @@ static int reflog_write(const char *log_path, const char *oid_old,
 	git_buf_truncate(&log, log.size - 1); /* drop LF */
 
 	if (msg) {
-		if (strchr(msg, '\n')) {
-			git_buf_free(&log);
-			return git__throw(GIT_ERROR, "Reflog message cannot contain newline");
-		}
-
 		git_buf_putc(&log, '\t');
 		git_buf_puts(&log, msg);
 	}
 
+	if (!trailing_newline)
 		git_buf_putc(&log, '\n');
 
 	if (git_buf_oom(&log)) {
 		git_buf_free(&log);
-		return git__throw(GIT_ENOMEM, "Failed to write reflog. Memory allocation failure");
-	}
-
-	if ((error = git_filebuf_open(&fbuf, log_path, GIT_FILEBUF_APPEND)) < GIT_SUCCESS) {
-		git_buf_free(&log);
-		return git__rethrow(error, "Failed to write reflog. Cannot open reflog `%s`", log_path);
+		return -1;
 	}
 
-	git_filebuf_write(&fbuf, log.ptr, log.size);
+	error = git_filebuf_open(&fbuf, log_path, GIT_FILEBUF_APPEND);
+	if (!error) {
+		if ((error = git_filebuf_write(&fbuf, log.ptr, log.size)) < 0)
+			git_filebuf_cleanup(&fbuf);
+		else
 			error = git_filebuf_commit(&fbuf, GIT_REFLOG_FILE_MODE);
+	}
 
 	git_buf_free(&log);
 
-	return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to write reflog");
+	return error;
 }
 
 static int reflog_parse(git_reflog *log, const char *buf, size_t buf_size)
 {
-	int error = GIT_SUCCESS;
 	const char *ptr;
 	git_reflog_entry *entry;
 
-#define seek_forward(_increase) { \
+#define seek_forward(_increase) do { \
 	if (_increase >= buf_size) { \
-		if (entry->committer) \
-			git__free(entry->committer); \
-		git__free(entry); \
-		return git__throw(GIT_ERROR, "Failed to seek forward. Buffer size exceeded"); \
+		giterr_set(GITERR_INVALID, "Ran out of data while parsing reflog"); \
+		goto fail; \
 	} \
 	buf += _increase; \
 	buf_size -= _increase; \
-}
+	} while (0)
 
 	while (buf_size > GIT_REFLOG_SIZE_MIN) {
 		entry = git__malloc(sizeof(git_reflog_entry));
-		if (entry == NULL)
-			return GIT_ENOMEM;
-		entry->committer = NULL;
+		GITERR_CHECK_ALLOC(entry);
 
-		if (git_oid_fromstrn(&entry->oid_old, buf, GIT_OID_HEXSZ) < GIT_SUCCESS) {
-			git__free(entry);
-			return GIT_ERROR;
-		}
+		entry->committer = git__malloc(sizeof(git_signature));
+		GITERR_CHECK_ALLOC(entry->committer);
+
+		if (git_oid_fromstrn(&entry->oid_old, buf, GIT_OID_HEXSZ) < 0)
+			goto fail;
 		seek_forward(GIT_OID_HEXSZ + 1);
 
-		if (git_oid_fromstrn(&entry->oid_cur, buf, GIT_OID_HEXSZ) < GIT_SUCCESS) {
-			git__free(entry);
-			return GIT_ERROR;
-		}
+		if (git_oid_fromstrn(&entry->oid_cur, buf, GIT_OID_HEXSZ) < 0)
+			goto fail;
 		seek_forward(GIT_OID_HEXSZ + 1);
 
 		ptr = buf;
@@ -124,17 +125,8 @@ static int reflog_parse(git_reflog *log, const char *buf, size_t buf_size)
 		while (*buf && *buf != '\t' && *buf != '\n')
 			seek_forward(1);
 
-		entry->committer = git__malloc(sizeof(git_signature));
-		if (entry->committer == NULL) {
-			git__free(entry);
-			return GIT_ENOMEM;
-		}
-
-		if ((error = git_signature__parse(entry->committer, &ptr, buf + 1, NULL, *buf)) < GIT_SUCCESS) {
-			git__free(entry->committer);
-			git__free(entry);
-			return git__rethrow(error, "Failed to parse reflog. Could not parse signature");
-		}
+		if (git_signature__parse(entry->committer, &ptr, buf + 1, NULL, *buf) < 0)
+			goto fail;
 
 		if (*buf == '\t') {
 			/* We got a message. Read everything till we reach LF. */
@@ -145,19 +137,27 @@ static int reflog_parse(git_reflog *log, const char *buf, size_t buf_size)
 				seek_forward(1);
 
 			entry->msg = git__strndup(ptr, buf - ptr);
+			GITERR_CHECK_ALLOC(entry->msg);
 		} else
 			entry->msg = NULL;
 
 		while (*buf && *buf == '\n' && buf_size > 1)
 			seek_forward(1);
 
-		if ((error = git_vector_insert(&log->entries, entry)) < GIT_SUCCESS)
-			return git__rethrow(error, "Failed to parse reflog. Could not add new entry");
+		if (git_vector_insert(&log->entries, entry) < 0)
+			goto fail;
 	}
 
+	return 0;
+
 #undef seek_forward
 
-	return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to parse reflog");
+fail:
+	if (entry) {
+		git__free(entry->committer);
+		git__free(entry);
+	}
+	return -1;
 }
 
 void git_reflog_free(git_reflog *reflog)
@@ -188,27 +188,23 @@ int git_reflog_read(git_reflog **reflog, git_reference *ref)
 
 	*reflog = NULL;
 
-	if ((error = reflog_init(&log, ref)) < GIT_SUCCESS)
-		return git__rethrow(error, "Failed to read reflog. Cannot init reflog");
+	if (reflog_init(&log, ref) < 0)
+		return -1;
 
 	error = git_buf_join_n(&log_path, '/', 3,
 		ref->owner->path_repository, GIT_REFLOG_DIR, ref->name);
-	if (error < GIT_SUCCESS)
-		goto cleanup;
 
-	if ((error = git_futils_readbuffer(&log_file, log_path.ptr)) < GIT_SUCCESS) {
-		git__rethrow(error, "Failed to read reflog. Cannot read file `%s`", log_path.ptr);
-		goto cleanup;
-	}
+	if (!error)
+		error = git_futils_readbuffer(&log_file, log_path.ptr);
 
-	if ((error = reflog_parse(log, log_file.ptr, log_file.size)) < GIT_SUCCESS)
-		git__rethrow(error, "Failed to read reflog");
-	else
-		*reflog = log;
+	if (!error)
+		error = reflog_parse(log, log_file.ptr, log_file.size);
 
-cleanup:
-	if (error != GIT_SUCCESS && log != NULL)
+	if (!error)
+		*reflog = log;
+	else
 		git_reflog_free(log);
+
 	git_buf_free(&log_file);
 	git_buf_free(&log_path);
 
@@ -225,16 +221,15 @@ int git_reflog_write(git_reference *ref, const git_oid *oid_old,
 	git_reference *r;
 	const git_oid *oid;
 
-	if ((error = git_reference_resolve(&r, ref)) < GIT_SUCCESS)
-		return git__rethrow(error,
-			"Failed to write reflog. Cannot resolve reference `%s`", ref->name);
+	if ((error = git_reference_resolve(&r, ref)) < 0)
+		return error;
 
 	oid = git_reference_oid(r);
 	if (oid == NULL) {
-		error = git__throw(GIT_ERROR,
+		giterr_set(GITERR_REFERENCE,
 			"Failed to write reflog. Cannot resolve reference `%s`", r->name);
 		git_reference_free(r);
-		return error;
+		return -1;
 	}
 
 	git_oid_to_string(new, GIT_OID_HEXSZ+1, oid);
@@ -243,23 +238,21 @@ int git_reflog_write(git_reference *ref, const git_oid *oid_old,
 
 	error = git_buf_join_n(&log_path, '/', 3,
 		ref->owner->path_repository, GIT_REFLOG_DIR, ref->name);
-	if (error < GIT_SUCCESS)
+	if (error < 0)
 		goto cleanup;
 
 	if (git_path_exists(log_path.ptr) == false) {
 		error = git_futils_mkpath2file(log_path.ptr, GIT_REFLOG_DIR_MODE);
-		if (error < GIT_SUCCESS)
-			git__rethrow(error,
-				"Failed to write reflog. Cannot create reflog directory");
 	} else if (git_path_isfile(log_path.ptr) == false) {
-		error = git__throw(GIT_ERROR,
+		giterr_set(GITERR_REFERENCE,
 			"Failed to write reflog. `%s` is directory", log_path.ptr);
+		error = -1;
 	} else if (oid_old == NULL) {
-		error = git__throw(GIT_ERROR,
+		giterr_set(GITERR_REFERENCE,
 			"Failed to write reflog. Old OID cannot be NULL for existing reference");
+		error = -1;
 	}
-
-	if (error < GIT_SUCCESS)
+	if (error < 0)
 		goto cleanup;
 
 	if (oid_old)
@@ -280,13 +273,13 @@ int git_reflog_rename(git_reference *ref, const char *new_name)
 	git_buf old_path = GIT_BUF_INIT;
 	git_buf new_path = GIT_BUF_INIT;
 
-	if (git_buf_join_n(&old_path, '/', 3, ref->owner->path_repository,
+	if (!git_buf_join_n(&old_path, '/', 3, ref->owner->path_repository,
 			GIT_REFLOG_DIR, ref->name) &&
-		git_buf_join_n(&new_path, '/', 3, ref->owner->path_repository,
+		!git_buf_join_n(&new_path, '/', 3, ref->owner->path_repository,
 			GIT_REFLOG_DIR, new_name))
 		error = p_rename(git_buf_cstr(&old_path), git_buf_cstr(&new_path));
 	else
-		error = GIT_ENOMEM;
+		error = -1;
 
 	git_buf_free(&old_path);
 	git_buf_free(&new_path);
@@ -296,13 +289,13 @@ int git_reflog_rename(git_reference *ref, const char *new_name)
 
 int git_reflog_delete(git_reference *ref)
 {
-	int error = GIT_SUCCESS;
+	int error;
 	git_buf path = GIT_BUF_INIT;
 
-	error = git_buf_join_n(&path, '/', 3,
-		ref->owner->path_repository, GIT_REFLOG_DIR, ref->name);
+	error = git_buf_join_n(
+		&path, '/', 3, ref->owner->path_repository, GIT_REFLOG_DIR, ref->name);
 
-	if (error == GIT_SUCCESS && git_path_exists(path.ptr) == true)
+	if (!error && git_path_exists(path.ptr))
 		error = p_unlink(path.ptr);
 
 	git_buf_free(&path);

src/signature.c

@@ -258,7 +258,7 @@ int git_signature__parse(git_signature *sig, const char **buffer_out,
 {
 	const char *buffer = *buffer_out;
 	const char *line_end, *name_end, *email_end, *tz_start, *time_start;
-	int error = GIT_SUCCESS;
+	int error = 0;
 
 	memset(sig, 0x0, sizeof(git_signature));
 

tests-clar/core/buffer.c

@@ -544,3 +544,20 @@ void test_core_buffer__9(void)
 
 	git_buf_free(&buf);
 }
+
+void test_core_buffer__10(void)
+{
+	git_buf a = GIT_BUF_INIT;
+
+	cl_git_pass(git_buf_join_n(&a, '/', 1, "test"));
+	cl_assert_strequal(a.ptr, "test");
+	cl_git_pass(git_buf_join_n(&a, '/', 1, "string"));
+	cl_assert_strequal(a.ptr, "test/string");
+	git_buf_clear(&a);
+	cl_git_pass(git_buf_join_n(&a, '/', 3, "test", "string", "join"));
+	cl_assert_strequal(a.ptr, "test/string/join");
+	cl_git_pass(git_buf_join_n(&a, '/', 2, a.ptr, "more"));
+	cl_assert_strequal(a.ptr, "test/string/join/test/string/join/more");
+
+	git_buf_free(&a);
+}