commit: extract the option parsing into its own function

Author: carlosmn

ext/rugged/rugged_commit.c

@@ -338,86 +338,60 @@ static VALUE rb_git_commit_amend(VALUE self, VALUE rb_data)
 	return rugged_create_oid(&commit_oid);
 }
 
-/*
- *  call-seq:
- *    Commit.create(repository, data = {}) -> oid
- *
- *  Write a new +Commit+ object to +repository+, with the given +data+
- *  arguments, passed as a +Hash+:
- *
- *  - +:message+: a string with the full text for the commit's message
- *  - +:committer+ (optional): a hash with the signature for the committer,
- *    defaults to the signature from the configuration
- *  - +:author+ (optional): a hash with the signature for the author,
- *    defaults to the signature from the configuration
- *  - +:parents+: an +Array+ with zero or more parents for this commit,
- *    represented as <tt>Rugged::Commit</tt> instances, or OID +String+.
- *  - +:tree+: the tree for this commit, represented as a <tt>Rugged::Tree</tt>
- *    instance or an OID +String+.
- *  - +:update_ref+ (optional): a +String+ with the name of a reference in the
- *    repository which should be updated to point to this commit (e.g. "HEAD")
- *
- *  When the commit is successfully written to disk, its +oid+ will be
- *  returned as a hex +String+.
- *
- *    author = {:email=>"[email protected]", :time=>Time.now, :name=>"Vicent Mart\303\255"}
+struct commit_data {
+	VALUE rb_err_obj;
+
+	const char *update_ref;
+	const char *message;
+	git_tree *tree;
+	git_signature *author;
+	git_signature *committer;
+	int parent_count;
+	const git_commit **parents;
+};
+
+/**
+ * Parse the commit options into something we can re-use
  *
- *    Rugged::Commit.create(r,
- *      :author => author,
- *      :message => "Hello world\n\n",
- *      :committer => author,
- *      :parents => ["2cb831a8aea28b2c1b9c63385585b864e4d3bad1"],
- *      :tree => some_tree) #=> "f148106ca58764adc93ad4e2d6b1d168422b9796"
+ * Note that parents may be set even when the function errors, so make
+ * sure to free this data.
  */
-static VALUE rb_git_commit_create(VALUE self, VALUE rb_repo, VALUE rb_data)
+static VALUE parse_commit_options(struct commit_data *out, git_repository *repo, VALUE rb_data)
 {
 	VALUE rb_message, rb_tree, rb_parents, rb_ref;
-	VALUE rb_err_obj = Qnil;
-	int parent_count, i, error = 0;
-	const git_commit **parents = NULL;
-	git_commit **free_list = NULL;
-	git_tree *tree;
-	git_signature *author, *committer;
-	git_oid commit_oid;
-	git_repository *repo;
-	const char *update_ref = NULL;
-
-	Check_Type(rb_data, T_HASH);
-
-	rugged_check_repo(rb_repo);
-	Data_Get_Struct(rb_repo, git_repository, repo);
+	int error = 0, parent_count, i;
 
 	rb_ref = rb_hash_aref(rb_data, CSTR2SYM("update_ref"));
 	if (!NIL_P(rb_ref)) {
 		Check_Type(rb_ref, T_STRING);
-		update_ref = StringValueCStr(rb_ref);
+		out->update_ref = StringValueCStr(rb_ref);
 	}
 
 	rb_message = rb_hash_aref(rb_data, CSTR2SYM("message"));
 	Check_Type(rb_message, T_STRING);
+	out->message = StringValueCStr(rb_message);
 
-	committer = rugged_signature_get(
+	out->committer = rugged_signature_get(
 		rb_hash_aref(rb_data, CSTR2SYM("committer")), repo
 	);
 
-	author = rugged_signature_get(
+	out->author = rugged_signature_get(
 		rb_hash_aref(rb_data, CSTR2SYM("author")), repo
 	);
 
 	rb_parents = rb_hash_aref(rb_data, CSTR2SYM("parents"));
 	Check_Type(rb_parents, T_ARRAY);
 
 	rb_tree = rb_hash_aref(rb_data, CSTR2SYM("tree"));
-	tree = (git_tree *)rugged_object_get(repo, rb_tree, GIT_OBJ_TREE);
+	out->tree = (git_tree *)rugged_object_get(repo, rb_tree, GIT_OBJ_TREE);
 
-	parents = alloca(RARRAY_LEN(rb_parents) * sizeof(void *));
-	free_list = alloca(RARRAY_LEN(rb_parents) * sizeof(void *));
+	out->parents = xcalloc(RARRAY_LEN(rb_parents), sizeof(void *));
 	parent_count = 0;
 
 	for (i = 0; i < (int)RARRAY_LEN(rb_parents); ++i) {
 		VALUE p = rb_ary_entry(rb_parents, i);
 		git_commit *parent = NULL;
-		git_commit *free_ptr = NULL;
+		git_commit *tmp = NULL;
 
 		if (NIL_P(p))
 			continue;
@@ -427,49 +401,106 @@ static VALUE rb_git_commit_create(VALUE self, VALUE rb_repo, VALUE rb_data)
 
 			error = git_oid_fromstr(&oid, StringValueCStr(p));
 			if (error < GIT_OK)
-				goto cleanup;
+				goto out;
 
 			error = git_commit_lookup(&parent, repo, &oid);
 			if (error < GIT_OK)
-				goto cleanup;
-
-			free_ptr = parent;
-
+				goto out;
 		} else if (rb_obj_is_kind_of(p, rb_cRuggedCommit)) {
-			Data_Get_Struct(p, git_commit, parent);
+			Data_Get_Struct(p, git_commit, tmp);
+			if ((error = git_object_dup((git_object **) &parent, (git_object *) tmp)) < 0)
+				goto out;
 		} else {
-			rb_err_obj = rb_exc_new2(rb_eTypeError, "Invalid type for parent object");
-			goto cleanup;
+			out->rb_err_obj = rb_exc_new2(rb_eTypeError, "Invalid type for parent object");
+			error = -1;
+			goto out;
 		}
 
-		parents[parent_count] = parent;
-		free_list[parent_count] = free_ptr;
+		out->parents[parent_count] = parent;
 		parent_count++;
 	}
 
+out:
+	out->parent_count = parent_count;
+	return error;
+}
+
+static void free_commit_options(struct commit_data *commit_data)
+{
+	int i;
+
+	git_signature_free(commit_data->author);
+	git_signature_free(commit_data->committer);
+
+	git_object_free((git_object *)commit_data->tree);
+
+	for (i = 0; i < commit_data->parent_count; ++i)
+		git_object_free((git_object *) commit_data->parents[i]);
+	xfree(commit_data->parents);
+}
+
+/*
+ *  call-seq:
+ *    Commit.create(repository, data = {}) -> oid
+ *
+ *  Write a new +Commit+ object to +repository+, with the given +data+
+ *  arguments, passed as a +Hash+:
+ *
+ *  - +:message+: a string with the full text for the commit's message
+ *  - +:committer+ (optional): a hash with the signature for the committer,
+ *    defaults to the signature from the configuration
+ *  - +:author+ (optional): a hash with the signature for the author,
+ *    defaults to the signature from the configuration
+ *  - +:parents+: an +Array+ with zero or more parents for this commit,
+ *    represented as <tt>Rugged::Commit</tt> instances, or OID +String+.
+ *  - +:tree+: the tree for this commit, represented as a <tt>Rugged::Tree</tt>
+ *    instance or an OID +String+.
+ *  - +:update_ref+ (optional): a +String+ with the name of a reference in the
+ *    repository which should be updated to point to this commit (e.g. "HEAD")
+ *
+ *  When the commit is successfully written to disk, its +oid+ will be
+ *  returned as a hex +String+.
+ *
+ *    author = {:email=>"[email protected]", :time=>Time.now, :name=>"Vicent Mart\303\255"}
+ *
+ *    Rugged::Commit.create(r,
+ *      :author => author,
+ *      :message => "Hello world\n\n",
+ *      :committer => author,
+ *      :parents => ["2cb831a8aea28b2c1b9c63385585b864e4d3bad1"],
+ *      :tree => some_tree) #=> "f148106ca58764adc93ad4e2d6b1d168422b9796"
+ */
+static VALUE rb_git_commit_create(VALUE self, VALUE rb_repo, VALUE rb_data)
+{
+	int error = 0;
+	struct commit_data commit_data = { Qnil };
+	git_oid commit_oid;
+	git_repository *repo;
+
+	Check_Type(rb_data, T_HASH);
+
+	rugged_check_repo(rb_repo);
+	Data_Get_Struct(rb_repo, git_repository, repo);
+
+	if ((error = parse_commit_options(&commit_data, repo, rb_data)) < 0)
+		goto cleanup;
+
 	error = git_commit_create(
 		&commit_oid,
 		repo,
-		update_ref,
-		author,
-		committer,
+		commit_data.update_ref,
+		commit_data.author,
+		commit_data.committer,
 		NULL,
-		StringValueCStr(rb_message),
-		tree,
-		parent_count,
-		parents);
+		commit_data.message,
+		commit_data.tree,
+		commit_data.parent_count,
+		commit_data.parents);
 
 cleanup:
-	git_signature_free(author);
-	git_signature_free(committer);
-
-	git_object_free((git_object *)tree);
-
-	for (i = 0; i < parent_count; ++i)
-		git_object_free((git_object *)free_list[i]);
-
-	if (!NIL_P(rb_err_obj))
-		rb_exc_raise(rb_err_obj);
+	free_commit_options(&commit_data);
+	if (!NIL_P(commit_data.rb_err_obj))
+		rb_exc_raise(commit_data.rb_err_obj);
 
 	rugged_exception_check(error);