[PATCH 0/2] Fix issues found by gcc 4.6.0

[PATCH 0/2] Fix issues found by gcc 4.6.0

[Johannes Schindelin]

In general, our sources are quite clean. However, a few variables were
unused. Previous gcc version did not find them because they were assigned
values, but those values were never used nontheless.

The tricky question is whether 2/2 really fixes an issue or not.
Unfortunately, my time is too limited to go into detail, but I figured
it'd be better to raise the issue than to remain silent.

Johannes Schindelin (2):
  Remove unused variables
  Actually use retval

 builtin/fsck.c       |    3 +--
 builtin/remote-ext.c |    4 ----
 diff.c               |    3 +--
 merge-recursive.c    |    4 ----
 reachable.c          |    5 -----
 test-subprocess.c    |    3 +--
 transport-helper.c   |    3 +--
 tree-diff.c          |    2 +-
 8 files changed, 5 insertions(+), 22 deletions(-)

-- 
1.7.4.msysgit.0.167.ge91fd

[PATCH 1/2] Remove unused variables

[Johannes Schindelin]

Noticed by gcc 4.6.0.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/fsck.c       |    3 +--
 builtin/remote-ext.c |    4 ----
 diff.c               |    3 +--
 merge-recursive.c    |    4 ----
 reachable.c          |    5 -----
 test-subprocess.c    |    3 +--
 transport-helper.c   |    3 +--
 7 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 795aba0..5ae0366 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -140,11 +140,10 @@ static int traverse_reachable(void)
 	int result = 0;
 	while (pending.nr) {
 		struct object_array_entry *entry;
-		struct object *obj, *parent;
+		struct object *obj;
 
 		entry = pending.objects + --pending.nr;
 		obj = entry->item;
-		parent = (struct object *) entry->name;
 		result |= traverse_one_object(obj);
 	}
 	return !!result;
diff --git a/builtin/remote-ext.c b/builtin/remote-ext.c
index ea71977..2343030 100644
--- a/builtin/remote-ext.c
+++ b/builtin/remote-ext.c
@@ -30,16 +30,12 @@ static char *strip_escapes(const char *str, const char *service,
 	size_t rpos = 0;
 	int escape = 0;
 	char special = 0;
-	size_t pslen = 0;
-	size_t pSlen = 0;
 	size_t psoff = 0;
 	struct strbuf ret = STRBUF_INIT;
 
 	/* Calculate prefix length for \s and lengths for \s and \S */
 	if (!strncmp(service, "git-", 4))
 		psoff = 4;
-	pSlen = strlen(service);
-	pslen = pSlen - psoff;
 
 	/* Pass the service to command. */
 	setenv("GIT_EXT_SERVICE", service, 1);
diff --git a/diff.c b/diff.c
index 42a107c..18f8e99 100644
--- a/diff.c
+++ b/diff.c
@@ -1242,7 +1242,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	uintmax_t max_change = 0, max_len = 0;
 	int total_files = data->nr;
 	int width, name_width;
-	const char *reset, *set, *add_c, *del_c;
+	const char *reset, *add_c, *del_c;
 	const char *line_prefix = "";
 	struct strbuf *msg = NULL;
 
@@ -1269,7 +1269,6 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 
 	/* Find the longest filename and max number of changes */
 	reset = diff_get_color_opt(options, DIFF_RESET);
-	set   = diff_get_color_opt(options, DIFF_PLAIN);
 	add_c = diff_get_color_opt(options, DIFF_FILE_NEW);
 	del_c = diff_get_color_opt(options, DIFF_FILE_OLD);
 
diff --git a/merge-recursive.c b/merge-recursive.c
index c8343d7..7c12673 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -357,7 +357,6 @@ static void make_room_for_directories_of_df_conflicts(struct merge_options *o,
 	 */
 	const char *last_file = NULL;
 	int last_len = 0;
-	struct stage_data *last_e;
 	int i;
 
 	/*
@@ -394,7 +393,6 @@ static void make_room_for_directories_of_df_conflicts(struct merge_options *o,
 		if (S_ISREG(e->stages[2].mode) || S_ISLNK(e->stages[2].mode)) {
 			last_file = path;
 			last_len = len;
-			last_e = e;
 		} else {
 			last_file = NULL;
 		}
@@ -969,7 +967,6 @@ static int process_renames(struct merge_options *o,
 	}
 
 	for (i = 0, j = 0; i < a_renames->nr || j < b_renames->nr;) {
-		char *src;
 		struct string_list *renames1, *renames2Dst;
 		struct rename *ren1 = NULL, *ren2 = NULL;
 		const char *branch1, *branch2;
@@ -1004,7 +1001,6 @@ static int process_renames(struct merge_options *o,
 			ren2 = ren1;
 			ren1 = tmp;
 		}
-		src = ren1->pair->one->path;
 
 		ren1->dst_entry->processed = 1;
 		ren1->src_entry->processed = 1;
diff --git a/reachable.c b/reachable.c
index a03fabf..3fc6b1d 100644
--- a/reachable.c
+++ b/reachable.c
@@ -70,16 +70,11 @@ static void process_tree(struct tree *tree,
 static void process_tag(struct tag *tag, struct object_array *p, const char *name)
 {
 	struct object *obj = &tag->object;
-	struct name_path me;
 
 	if (obj->flags & SEEN)
 		return;
 	obj->flags |= SEEN;
 
-	me.up = NULL;
-	me.elem = "tag:/";
-	me.elem_len = 5;
-
 	if (parse_tag(tag) < 0)
 		die("bad tag object %s", sha1_to_hex(obj->sha1));
 	if (tag->tagged)
diff --git a/test-subprocess.c b/test-subprocess.c
index 667d3e5..8926bc5 100644
--- a/test-subprocess.c
+++ b/test-subprocess.c
@@ -3,11 +3,10 @@
 
 int main(int argc, char **argv)
 {
-	const char *prefix;
 	struct child_process cp;
 	int nogit = 0;
 
-	prefix = setup_git_directory_gently(&nogit);
+	setup_git_directory_gently(&nogit);
 	if (nogit)
 		die("No git repo found");
 	if (!strcmp(argv[1], "--setup-work-tree")) {
diff --git a/transport-helper.c b/transport-helper.c
index 0c5b1bd..a4f40fb 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -561,10 +561,9 @@ static int push_refs_with_push(struct transport *transport,
 	int mirror = flags & TRANSPORT_PUSH_MIRROR;
 	struct helper_data *data = transport->data;
 	struct strbuf buf = STRBUF_INIT;
-	struct child_process *helper;
 	struct ref *ref;
 
-	helper = get_helper(transport);
+	get_helper(transport);
 	if (!data->push)
 		return 1;
 
-- 
1.7.4.msysgit.0.167.ge91fd

Re: [PATCH 1/2] Remove unused variables

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> Noticed by gcc 4.6.0.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---

Thanks.

I'll cherry pick only some bits, not because the changes in this patch are
wrong, but because I know some areas are in flux and I don't want to deal
with merge conflicts of the "one side fixed a small bug, the other side
majorly rewrote it and made the bug irrelevant" kind.

Re: [PATCH 1/2] Remove unused variables

[Johannes Schindelin]

Hi,

On Tue, 22 Mar 2011, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > Noticed by gcc 4.6.0.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> 
> Thanks.
> 
> I'll cherry pick only some bits, not because the changes in this patch 
> are wrong, but because I know some areas are in flux and I don't want to 
> deal with merge conflicts of the "one side fixed a small bug, the other 
> side majorly rewrote it and made the bug irrelevant" kind.

Understandable. I will have to deal with the merge conflicts, though, as I 
am stuck with 4.6.0 (and self-imposed -Werror, as you might remember) for 
the 64-bit Windows build. So I will keep the remains of the patch after 
rebasing in our 'devel' branch.

Ciao,
Johannes

Re: [PATCH 1/2] Remove unused variables

[Jonathan Nieder]

Hi,

These all look good.  Some notes:

Johannes Schindelin wrote:

> --- a/builtin/fsck.c
> +++ b/builtin/fsck.c
> @@ -140,11 +140,10 @@ static int traverse_reachable(void)
>  	int result = 0;
>  	while (pending.nr) {
>  		struct object_array_entry *entry;
> -		struct object *obj, *parent;
> +		struct object *obj;

Cleaning up after v1.7.4.1~1^2~1 (fsck: drop unused parameter from
traverse_one_object(), 2011-01-26).  Sensible.

> --- a/builtin/remote-ext.c
> +++ b/builtin/remote-ext.c
> @@ -30,16 +30,12 @@ static char *strip_escapes(const char *str, const char *service,
>  	size_t rpos = 0;
>  	int escape = 0;
>  	char special = 0;
> -	size_t pslen = 0;
> -	size_t pSlen = 0;

These were never used; sorry to have missed it before.

> --- a/diff.c
> +++ b/diff.c
> @@ -1242,7 +1242,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
>  	uintmax_t max_change = 0, max_len = 0;
>  	int total_files = data->nr;
>  	int width, name_width;
> -	const char *reset, *set, *add_c, *del_c;
> +	const char *reset, *add_c, *del_c;

After v1.6.4-rc0~159^2 (diff: do not color --stat output like patch
context, 2009-04-25), the "patch context" color is not used in
diffstats.

> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -357,7 +357,6 @@ static void make_room_for_directories_of_df_conflicts(struct merge_options *o,
>  	 */
>  	const char *last_file = NULL;
>  	int last_len = 0;
> -	struct stage_data *last_e;

Yep, was never used (and while the code to set it is correct, it
is better to remove it than to let it bitrot).

> @@ -969,7 +967,6 @@ static int process_renames(struct merge_options *o,
>  	}
>  
>  	for (i = 0, j = 0; i < a_renames->nr || j < b_renames->nr;) {
> -		char *src;

Likewise.

> --- a/reachable.c
> +++ b/reachable.c
> @@ -70,16 +70,11 @@ static void process_tree(struct tree *tree,
>  static void process_tag(struct tag *tag, struct object_array *p, const char *name)
>  {
>  	struct object *obj = &tag->object;
> -	struct name_path me;
>  
>  	if (obj->flags & SEEN)
>  		return;
>  	obj->flags |= SEEN;
>  
> -	me.up = NULL;
> -	me.elem = "tag:/";
> -	me.elem_len = 5;
> -
>  	if (parse_tag(tag) < 0)
>  		die("bad tag object %s", sha1_to_hex(obj->sha1));
>  	if (tag->tagged)

Does this code make sense at all?  It peels a tag once in order to
mark the object it refers to with "add_object", which adds the object
to the array referred to by "p", that nobody actually uses.  Maybe
this is a remnant of an earlier implementation for "git log
--objects"?

Peeling of tags is already taken care of by revision::handle_commit,
so I think the function could be stripped down to

 static void process_tag(struct tag *tag, struct object_array *p, const char *name)
 {
	struct object *obj = &tag->object;
 
	if (obj->flags & SEEN)
		return;
	obj->flags |= SEEN;
 }

which seems to work okay still.

> --- a/test-subprocess.c
> +++ b/test-subprocess.c
> @@ -3,11 +3,10 @@
[...]
> -	prefix = setup_git_directory_gently(&nogit);
> +	setup_git_directory_gently(&nogit);

It might be more idiomatic to use setup_git_directory, but this is
better because conservative.

> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -561,10 +561,9 @@ static int push_refs_with_push(struct transport *transport,
>  	int mirror = flags & TRANSPORT_PUSH_MIRROR;
>  	struct helper_data *data = transport->data;
>  	struct strbuf buf = STRBUF_INIT;
> -	struct child_process *helper;

Cleaning up after v1.7.0-rc0~62^2~12 (Add remote helper debug mode,
2009-12-09).  The remote helper process is accessed through
transport->data->helper.

Thanks and hope that helps.
Jonathan

[PATCH 2/2] Actually use retval

[Johannes Schindelin]

This is most likely a bug. Nocited by gcc 4.6.0.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Alternatively, retval's declaration and assignment can go.

 tree-diff.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tree-diff.c b/tree-diff.c
index 3954281..9e2dc70 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -58,7 +58,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2,
 		opt->change(opt, mode1, mode2, sha1, sha2, base->buf, 0, 0);
 	}
 	strbuf_setlen(base, old_baselen);
-	return 0;
+	return retval;
 }
 
 /* A whole sub-tree went away or appeared */
-- 
1.7.4.msysgit.0.167.ge91fd

Re: [PATCH 2/2] Actually use retval

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> This is most likely a bug. Nocited by gcc 4.6.0.

While I don't doubt gcc 4.6.0 found the retval assigned is not used, I
think you misunderstood the value returned from this function.

The caller uses the return value to decide if an entry from t1 (and not
from t2) was consumed, if an entry each from both t1 and t2 were consumed,
or an entry from t2 (and not from t1) was consumed.  It doesn't change the
fact that the entry at the beginning of each tree we looked at in this
function at that point shared the same name and we consumed them, whatever
the call to diff_tree_sha1() to run a recursive comparison between the
trees found.

The likely fix would be to remove assignment to retval instead.

Re: [PATCH 2/2] Actually use retval

[Johannes Schindelin]

Hi,

On Tue, 22 Mar 2011, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > This is most likely a bug. Nocited by gcc 4.6.0.
> 
> While I don't doubt gcc 4.6.0 found the retval assigned is not used, I 
> think you misunderstood the value returned from this function.

Nope. I did not misunderstand. I did not understand in the first place. 
That is what I described pretty explicitly in the cover letter.

> The caller uses the return value to decide if an entry from t1 (and not 
> from t2) was consumed, if an entry each from both t1 and t2 were 
> consumed, or an entry from t2 (and not from t1) was consumed.  It 
> doesn't change the fact that the entry at the beginning of each tree we 
> looked at in this function at that point shared the same name and we 
> consumed them, whatever the call to diff_tree_sha1() to run a recursive 
> comparison between the trees found.
> 
> The likely fix would be to remove assignment to retval instead.

Thanks, that is the alternative I suggested after the dashdashdash.

Ciao,
Johannes

[PATCH 0/2] Re: [PATCH 2/2] Actually use retval

[Jonathan Nieder]

Junio C Hamano wrote:

> The likely fix would be to remove assignment to retval instead.

Like this, maybe?

Jonathan Nieder (2):
  tree-diff: make diff_tree() return void
  tree-diff: make diff_tree_sha1() return void

 diff.h      |    4 ++--
 revision.c  |    9 +++------
 tree-diff.c |   17 ++++++-----------
 3 files changed, 11 insertions(+), 19 deletions(-)

[PATCH 1/2] tree-diff: make diff_tree return void

[Jonathan Nieder]

diff_tree (which runs a diff between two trees) always returns 0 and
dies on errors.  Some day it may change to change it to return -1 on
error but it will be easy to adjust callers then and until then the
return value is just confusing.

This way at least callers are consistent in ignoring the return value
and new readers don't have to wonder if diff_tree returns its diff
result like "diff --exit-code" would.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 diff.h      |    2 +-
 revision.c  |    5 ++---
 tree-diff.c |   13 +++++--------
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/diff.h b/diff.h
index 007a055..4fde7f3 100644
--- a/diff.h
+++ b/diff.h
@@ -163,7 +163,7 @@ extern const char mime_boundary_leader[];
 
 extern void diff_tree_setup_paths(const char **paths, struct diff_options *);
 extern void diff_tree_release_paths(struct diff_options *);
-extern int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
+extern void diff_tree(struct tree_desc *t1, struct tree_desc *t2,
 		     const char *base, struct diff_options *opt);
 extern int diff_tree_sha1(const unsigned char *old, const unsigned char *new,
 			  const char *base, struct diff_options *opt);
diff --git a/revision.c b/revision.c
index 86d2470..a6e78c9 100644
--- a/revision.c
+++ b/revision.c
@@ -337,7 +337,6 @@ static int rev_compare_tree(struct rev_info *revs, struct commit *parent, struct
 
 static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit)
 {
-	int retval;
 	void *tree;
 	unsigned long size;
 	struct tree_desc empty, real;
@@ -354,10 +353,10 @@ static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit)
 
 	tree_difference = REV_TREE_SAME;
 	DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);
-	retval = diff_tree(&empty, &real, "", &revs->pruning);
+	diff_tree(&empty, &real, "", &revs->pruning);
 	free(tree);
 
-	return retval >= 0 && (tree_difference == REV_TREE_SAME);
+	return tree_difference == REV_TREE_SAME;
 }
 
 static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
diff --git a/tree-diff.c b/tree-diff.c
index 3954281..d1a7ae9 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -138,7 +138,7 @@ static void skip_uninteresting(struct tree_desc *t, struct strbuf *base,
 	}
 }
 
-int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
+void diff_tree(struct tree_desc *t1, struct tree_desc *t2,
 	      const char *base_str, struct diff_options *opt)
 {
 	struct strbuf base;
@@ -190,7 +190,6 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
 	}
 
 	strbuf_release(&base);
-	return 0;
 }
 
 /*
@@ -285,7 +284,6 @@ int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const cha
 	void *tree1, *tree2;
 	struct tree_desc t1, t2;
 	unsigned long size1, size2;
-	int retval;
 
 	tree1 = read_object_with_reference(old, tree_type, &size1, NULL);
 	if (!tree1)
@@ -295,7 +293,7 @@ int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const cha
 		die("unable to read destination tree (%s)", sha1_to_hex(new));
 	init_tree_desc(&t1, tree1, size1);
 	init_tree_desc(&t2, tree2, size2);
-	retval = diff_tree(&t1, &t2, base, opt);
+	diff_tree(&t1, &t2, base, opt);
 	if (!*base && DIFF_OPT_TST(opt, FOLLOW_RENAMES) && diff_might_be_rename()) {
 		init_tree_desc(&t1, tree1, size1);
 		init_tree_desc(&t2, tree2, size2);
@@ -303,12 +301,11 @@ int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const cha
 	}
 	free(tree1);
 	free(tree2);
-	return retval;
+	return 0;
 }
 
 int diff_root_tree_sha1(const unsigned char *new, const char *base, struct diff_options *opt)
 {
-	int retval;
 	void *tree;
 	unsigned long size;
 	struct tree_desc empty, real;
@@ -319,9 +316,9 @@ int diff_root_tree_sha1(const unsigned char *new, const char *base, struct diff_
 	init_tree_desc(&real, tree, size);
 
 	init_tree_desc(&empty, "", 0);
-	retval = diff_tree(&empty, &real, base, opt);
+	diff_tree(&empty, &real, base, opt);
 	free(tree);
-	return retval;
+	return 0;
 }
 
 void diff_tree_release_paths(struct diff_options *opt)
-- 
1.7.4.1

[PATCH 2/2] tree-diff: make diff_tree_sha1 return void

[Jonathan Nieder]

diff_tree_sha1 does not return -1 on errors; it die()s.  Now it would
obviously be lovely to change that so long-running programs could call
diff_tree_sha1 without fear, but until then, let's face the reality:

 - nobody has tested what would happen if it started returning
   instead of dying on failure;
 - the callers overwhelmingly ignore the return value.

Returning void also saves readers the trouble of checking
documentation each time they use diff_tree_sha1 to see if its return
value follows the GNU diff exit code convention.

So it's just more honest to return void.  Do so.

Noticed with gcc-4.6 -Wunused-but-set-variable (because one caller
couldn't decide --- it saves the return value, but never does anything
with it).

Noticed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 diff.h      |    2 +-
 revision.c  |    4 +---
 tree-diff.c |    6 ++----
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/diff.h b/diff.h
index 4fde7f3..0a8a06c 100644
--- a/diff.h
+++ b/diff.h
@@ -165,7 +165,7 @@ extern void diff_tree_setup_paths(const char **paths, struct diff_options *);
 extern void diff_tree_release_paths(struct diff_options *);
 extern void diff_tree(struct tree_desc *t1, struct tree_desc *t2,
 		     const char *base, struct diff_options *opt);
-extern int diff_tree_sha1(const unsigned char *old, const unsigned char *new,
+extern void diff_tree_sha1(const unsigned char *old, const unsigned char *new,
 			  const char *base, struct diff_options *opt);
 extern int diff_root_tree_sha1(const unsigned char *new, const char *base,
                                struct diff_options *opt);
diff --git a/revision.c b/revision.c
index a6e78c9..0c810b4 100644
--- a/revision.c
+++ b/revision.c
@@ -329,9 +329,7 @@ static int rev_compare_tree(struct rev_info *revs, struct commit *parent, struct
 
 	tree_difference = REV_TREE_SAME;
 	DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);
-	if (diff_tree_sha1(t1->object.sha1, t2->object.sha1, "",
-			   &revs->pruning) < 0)
-		return REV_TREE_DIFFERENT;
+	diff_tree_sha1(t1->object.sha1, t2->object.sha1, "", &revs->pruning);
 	return tree_difference;
 }
 
diff --git a/tree-diff.c b/tree-diff.c
index d1a7ae9..03b8ca0 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -17,7 +17,6 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2,
 	const unsigned char *sha1, *sha2;
 	int cmp, pathlen1, pathlen2;
 	int old_baselen = base->len;
-	int retval = 0;
 
 	sha1 = tree_entry_extract(t1, &path1, &mode1);
 	sha2 = tree_entry_extract(t2, &path2, &mode2);
@@ -53,7 +52,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2,
 				    sha1, sha2, base->buf, 0, 0);
 		}
 		strbuf_addch(base, '/');
-		retval = diff_tree_sha1(sha1, sha2, base->buf, opt);
+		diff_tree_sha1(sha1, sha2, base->buf, opt);
 	} else {
 		opt->change(opt, mode1, mode2, sha1, sha2, base->buf, 0, 0);
 	}
@@ -279,7 +278,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
 	q->nr = 1;
 }
 
-int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const char *base, struct diff_options *opt)
+void diff_tree_sha1(const unsigned char *old, const unsigned char *new, const char *base, struct diff_options *opt)
 {
 	void *tree1, *tree2;
 	struct tree_desc t1, t2;
@@ -301,7 +300,6 @@ int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const cha
 	}
 	free(tree1);
 	free(tree2);
-	return 0;
 }
 
 int diff_root_tree_sha1(const unsigned char *new, const char *base, struct diff_options *opt)
-- 
1.7.4.1