Merge pull request #348 from schu/sig-new

signature.c: fix off-by-one error

include/git2/signature.h

@@ -41,23 +41,25 @@ GIT_BEGIN_DECL
  * Create a new action signature. The signature must be freed
  * manually or using git_signature_free
  *
+ * @param sig_out new signature, in case of error NULL
  * @param name name of the person
  * @param email email of the person
  * @param time time when the action happened
  * @param offset timezone offset in minutes for the time
- * @return the new sig, NULL on out of memory
+ * @return 0 on success; error code otherwise
  */
-GIT_EXTERN(git_signature *) git_signature_new(const char *name, const char *email, git_time_t time, int offset);
+GIT_EXTERN(int) git_signature_new(git_signature **sig_out, const char *name, const char *email, git_time_t time, int offset);
 
 /**
  * Create a new action signature with a timestamp of 'now'. The
  * signature must be freed manually or using git_signature_free
  *
+ * @param sig_out new signature, in case of error NULL
  * @param name name of the person
  * @param email email of the person
- * @return the new sig, NULL on out of memory
+ * @return 0 on success; error code otherwise
  */
-GIT_EXTERN(git_signature *) git_signature_now(const char *name, const char *email);
+GIT_EXTERN(int) git_signature_now(git_signature **sig_out, const char *name, const char *email);
 
 
 /**

src/signature.c

@@ -62,7 +62,7 @@ static int process_trimming(const char *input, char **storage, const char *input
 	left = skip_leading_spaces(input, input_end);
 	right = skip_trailing_spaces(input, input_end - 1);
 
-	if (right <= left) {
+	if (right < left) {
 		if (fail_when_empty)
 			return git__throw(GIT_EINVALIDARGS, "Failed to trim. Input is either empty or only contains spaces");
 		else
@@ -81,15 +81,19 @@ static int process_trimming(const char *input, char **storage, const char *input
 	return GIT_SUCCESS;
 }
 
-git_signature *git_signature_new(const char *name, const char *email, git_time_t time, int offset)
+int git_signature_new(git_signature **sig_out, const char *name, const char *email, git_time_t time, int offset)
 {
 	int error;
 	git_signature *p = NULL;
 
 	assert(name && email);
 
-	if ((p = git__malloc(sizeof(git_signature))) == NULL)
+	*sig_out = NULL;
+
+	if ((p = git__malloc(sizeof(git_signature))) == NULL) {
+		error = GIT_ENOMEM;
 		goto cleanup;
+	}
 
 	memset(p, 0x0, sizeof(git_signature));
 
@@ -108,28 +112,37 @@ git_signature *git_signature_new(const char *name, const char *email, git_time_t
 	p->when.time = time;
 	p->when.offset = offset;
 
-	return p;
+	*sig_out = p;
+
+	return error;
 
 cleanup:
 	git_signature_free(p);
-	return NULL;
+	return error;
 }
 
 git_signature *git_signature_dup(const git_signature *sig)
 {
-	return git_signature_new(sig->name, sig->email, sig->when.time, sig->when.offset);
+	git_signature *new;
+	if (git_signature_new(&new, sig->name, sig->email, sig->when.time, sig->when.offset) < GIT_SUCCESS)
+		return NULL;
+	return new;
 }
 
-git_signature *git_signature_now(const char *name, const char *email)
+int git_signature_now(git_signature **sig_out, const char *name, const char *email)
 {
+	int error;
 	time_t now;
 	time_t offset;
 	struct tm *utc_tm, *local_tm;
+	git_signature *sig;
 
 #ifndef GIT_WIN32
 	struct tm _utc, _local;
 #endif
 
+	*sig_out = NULL;
+
 	time(&now);
 
 	/**
@@ -151,7 +164,12 @@ git_signature *git_signature_now(const char *name, const char *email)
 	if (local_tm->tm_isdst)
 		offset += 60;
 
-	return git_signature_new(name, email, now, (int)offset);
+	if ((error = git_signature_new(&sig, name, email, now, (int)offset)) < GIT_SUCCESS)
+		return error;
+
+	*sig_out = sig;
+
+	return error;
 }
 
 static int parse_timezone_offset(const char *buffer, int *offset_out)

tests/t04-commit.c

@@ -450,10 +450,8 @@ static int try_build_signature(const char *name, const char *email, git_time_t t
 	git_signature *sign;
 	int error = GIT_SUCCESS;
 
-	sign = git_signature_new(name, email, time, offset);
-
-	if (sign == NULL)
-		error = GIT_ERROR;
+	if ((error =  git_signature_new(&sign, name, email, time, offset)) < GIT_SUCCESS)
+		return error;
 
 	git_signature_free((git_signature *)sign);
 
@@ -462,8 +460,7 @@ static int try_build_signature(const char *name, const char *email, git_time_t t
 
 BEGIN_TEST(signature0, "creating a signature trims leading and trailing spaces")
 	git_signature *sign;
-	sign = git_signature_new("  nulltoken ", "   emeric.fermas@gmail.com     ", 1234567890, 60);
-	must_be_true(sign != NULL);
+	must_pass(git_signature_new(&sign, "  nulltoken ", "   emeric.fermas@gmail.com     ", 1234567890, 60));
 	must_pass(strcmp(sign->name, "nulltoken"));
 	must_pass(strcmp(sign->email, "emeric.fermas@gmail.com"));
 	git_signature_free((git_signature *)sign);
@@ -478,6 +475,29 @@ BEGIN_TEST(signature1, "can not create a signature with empty name or email")
 	must_fail(try_build_signature("nulltoken", "  ", 1234567890, 60));
 END_TEST
 
+BEGIN_TEST(signature2, "creating a one character signature")
+	git_signature *sign;
+	must_pass(git_signature_new(&sign, "x", "foo@bar.baz", 1234567890, 60));
+	must_pass(strcmp(sign->name, "x"));
+	must_pass(strcmp(sign->email, "foo@bar.baz"));
+	git_signature_free((git_signature *)sign);
+END_TEST
+
+BEGIN_TEST(signature3, "creating a two character signature")
+	git_signature *sign;
+	must_pass(git_signature_new(&sign, "xx", "x@y.z", 1234567890, 60));
+	must_pass(strcmp(sign->name, "x"));
+	must_pass(strcmp(sign->email, "foo@bar.baz"));
+	git_signature_free((git_signature *)sign);
+END_TEST
+
+BEGIN_TEST(signature4, "creating a zero character signature")
+	git_signature *sign;
+	must_fail(git_signature_new(&sign, "", "x@y.z", 1234567890, 60));
+	must_be_true(sign == NULL);
+END_TEST
+
+
 /* External declaration for testing the buffer parsing method */
 int commit_parse_buffer(git_commit *commit, void *data, size_t len, unsigned int parse_flags);
 
@@ -629,11 +649,8 @@ BEGIN_TEST(write0, "write a new commit object from memory to disk")
 	must_pass(git_commit_lookup(&parent, repo, &parent_id));
 
 	/* create signatures */
-	committer = git_signature_new(COMMITTER_NAME, COMMITTER_EMAIL, 123456789, 60);
-	must_be_true(committer != NULL);
-
-	author = git_signature_new(COMMITTER_NAME, COMMITTER_EMAIL, 987654321, 90);
-	must_be_true(author != NULL);
+	must_pass(git_signature_new(&committer, COMMITTER_NAME, COMMITTER_EMAIL, 123456789, 60));
+	must_pass(git_signature_new(&author, COMMITTER_NAME, COMMITTER_EMAIL, 987654321, 90));
 
 	must_pass(git_commit_create_v(
 		&commit_id, /* out id */
@@ -696,11 +713,8 @@ BEGIN_TEST(root0, "create a root commit")
 	must_pass(git_tree_lookup(&tree, repo, &tree_id));
 
 	/* create signatures */
-	committer = git_signature_new(COMMITTER_NAME, COMMITTER_EMAIL, 123456789, 60);
-	must_be_true(committer != NULL);
-
-	author = git_signature_new(COMMITTER_NAME, COMMITTER_EMAIL, 987654321, 90);
-	must_be_true(author != NULL);
+	must_pass(git_signature_new(&committer, COMMITTER_NAME, COMMITTER_EMAIL, 123456789, 60));
+	must_pass(git_signature_new(&author, COMMITTER_NAME, COMMITTER_EMAIL, 987654321, 90));
 
 	/* First we need to update HEAD so it points to our non-existant branch */
 	must_pass(git_reference_lookup(&head, repo, "HEAD"));
@@ -758,4 +772,7 @@ BEGIN_SUITE(commit)
 
 	ADD_TEST(signature0);
 	ADD_TEST(signature1);
+	ADD_TEST(signature2);
+	ADD_TEST(signature3);
+	ADD_TEST(signature4);
 END_SUITE

tests/t08-tag.c

@@ -126,8 +126,7 @@ BEGIN_TEST(write0, "write a tag to the repository and read it again")
 	must_pass(git_object_lookup(&target, repo, &target_id, GIT_OBJ_COMMIT));
 
 	/* create signature */
-	tagger = git_signature_new(TAGGER_NAME, TAGGER_EMAIL, 123456789, 60);
-	must_be_true(tagger != NULL);
+	must_pass(git_signature_new(&tagger, TAGGER_NAME, TAGGER_EMAIL, 123456789, 60));
 
 	must_pass(git_tag_create(
 		&tag_id, /* out id */
@@ -177,8 +176,7 @@ BEGIN_TEST(write2, "Attempt to write a tag bearing the same name than an already
 	must_pass(git_object_lookup(&target, repo, &target_id, GIT_OBJ_COMMIT));
 
 	/* create signature */
-	tagger = git_signature_new(TAGGER_NAME, TAGGER_EMAIL, 123456789, 60);
-	must_be_true(tagger != NULL);
+	must_pass(git_signature_new(&tagger, TAGGER_NAME, TAGGER_EMAIL, 123456789, 60));
 
 	must_fail(git_tag_create(
 		&tag_id, /* out id */
@@ -212,8 +210,7 @@ BEGIN_TEST(write3, "Replace an already existing tag")
 	git_oid_cpy(&old_tag_id, git_reference_oid(ref_tag));
 
 	/* create signature */
-	tagger = git_signature_new(TAGGER_NAME, TAGGER_EMAIL, 123456789, 60);
-	must_be_true(tagger != NULL);
+	must_pass(git_signature_new(&tagger, TAGGER_NAME, TAGGER_EMAIL, 123456789, 60));
 
 	must_pass(git_tag_create(
 		&tag_id, /* out id */

tests/t10-refs.c

@@ -1034,7 +1034,7 @@ BEGIN_TEST(reflog0, "write a reflog for a given reference and ensure it can be r
 	must_pass(git_reference_create_oid(&ref, repo, new_ref, &oid, 0));
 	must_pass(git_reference_lookup(&ref, repo, new_ref));
 
-	committer = git_signature_now("foo", "foo@bar");
+	must_pass(git_signature_now(&committer, "foo", "foo@bar"));
 
 	must_pass(git_reflog_write(ref, NULL, committer, NULL));
 	must_fail(git_reflog_write(ref, NULL, committer, "no\nnewline"));
@@ -1082,7 +1082,7 @@ BEGIN_TEST(reflog1, "avoid writing an obviously wrong reflog")
 	must_pass(git_reference_create_oid(&ref, repo, new_ref, &oid, 0));
 	must_pass(git_reference_lookup(&ref, repo, new_ref));
 
-	committer = git_signature_now("foo", "foo@bar");
+	must_pass(git_signature_now(&committer, "foo", "foo@bar"));
 
 	/* Write the reflog for the new branch */
 	must_pass(git_reflog_write(ref, NULL, committer, NULL));