{"thread":{"id":"26826","subject":"[PATCH 0/2] Fix issues found by gcc 4.6.0","startedAt":"2011-03-22T12:49:56Z","lastAt":"2011-03-23T01:30:21Z","messageCount":11,"participants":["Johannes Schindelin","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"164041","messageId":"cover.1300798069.git.johannes.schindelin@gmx.de","threadId":"26826","inReplyTo":null,"subject":"[PATCH 0/2] Fix issues found by gcc 4.6.0","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2011-03-22T12:49:56Z","receivedAt":"2011-03-22T12:49:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In general, our sources are quite clean. However, a few variables were\nunused. Previous gcc version did not find them because they were assigned\nvalues, but those values were never used nontheless.\n\nThe tricky question is whether 2/2 really fixes an issue or not.\nUnfortunately, my time is too limited to go into detail, but I figured\nit'd be better to raise the issue than to remain silent.\n\nJohannes Schindelin (2):\n  Remove unused variables\n  Actually use retval\n\n builtin/fsck.c       |    3 +--\n builtin/remote-ext.c |    4 ----\n diff.c               |    3 +--\n merge-recursive.c    |    4 ----\n reachable.c          |    5 -----\n test-subprocess.c    |    3 +--\n transport-helper.c   |    3 +--\n tree-diff.c          |    2 +-\n 8 files changed, 5 insertions(+), 22 deletions(-)\n\n-- \n1.7.4.msysgit.0.167.ge91fd\n"},{"id":"164042","messageId":"2aeb4f915afaf712c1d5bba46f4010a325ea14c5.1300798069.git.johannes.schindelin@gmx.de","threadId":"26826","inReplyTo":"cover.1300798069.git.johannes.schindelin@gmx.de","subject":"[PATCH 1/2] Remove unused variables","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2011-03-22T12:50:08Z","receivedAt":"2011-03-22T12:50:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Noticed by gcc 4.6.0.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n builtin/fsck.c       |    3 +--\n builtin/remote-ext.c |    4 ----\n diff.c               |    3 +--\n merge-recursive.c    |    4 ----\n reachable.c          |    5 -----\n test-subprocess.c    |    3 +--\n transport-helper.c   |    3 +--\n 7 files changed, 4 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 795aba0..5ae0366 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -140,11 +140,10 @@ static int traverse_reachable(void)\n \tint result = 0;\n \twhile (pending.nr) {\n \t\tstruct object_array_entry *entry;\n-\t\tstruct object *obj, *parent;\n+\t\tstruct object *obj;\n \n \t\tentry = pending.objects + --pending.nr;\n \t\tobj = entry->item;\n-\t\tparent = (struct object *) entry->name;\n \t\tresult |= traverse_one_object(obj);\n \t}\n \treturn !!result;\ndiff --git a/builtin/remote-ext.c b/builtin/remote-ext.c\nindex ea71977..2343030 100644\n--- a/builtin/remote-ext.c\n+++ b/builtin/remote-ext.c\n@@ -30,16 +30,12 @@ static char *strip_escapes(const char *str, const char *service,\n \tsize_t rpos = 0;\n \tint escape = 0;\n \tchar special = 0;\n-\tsize_t pslen = 0;\n-\tsize_t pSlen = 0;\n \tsize_t psoff = 0;\n \tstruct strbuf ret = STRBUF_INIT;\n \n \t/* Calculate prefix length for \\s and lengths for \\s and \\S */\n \tif (!strncmp(service, \"git-\", 4))\n \t\tpsoff = 4;\n-\tpSlen = strlen(service);\n-\tpslen = pSlen - psoff;\n \n \t/* Pass the service to command. */\n \tsetenv(\"GIT_EXT_SERVICE\", service, 1);\ndiff --git a/diff.c b/diff.c\nindex 42a107c..18f8e99 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1242,7 +1242,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \tuintmax_t max_change = 0, max_len = 0;\n \tint total_files = data->nr;\n \tint width, name_width;\n-\tconst char *reset, *set, *add_c, *del_c;\n+\tconst char *reset, *add_c, *del_c;\n \tconst char *line_prefix = \"\";\n \tstruct strbuf *msg = NULL;\n \n@@ -1269,7 +1269,6 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \n \t/* Find the longest filename and max number of changes */\n \treset = diff_get_color_opt(options, DIFF_RESET);\n-\tset   = diff_get_color_opt(options, DIFF_PLAIN);\n \tadd_c = diff_get_color_opt(options, DIFF_FILE_NEW);\n \tdel_c = diff_get_color_opt(options, DIFF_FILE_OLD);\n \ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex c8343d7..7c12673 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -357,7 +357,6 @@ static void make_room_for_directories_of_df_conflicts(struct merge_options *o,\n \t */\n \tconst char *last_file = NULL;\n \tint last_len = 0;\n-\tstruct stage_data *last_e;\n \tint i;\n \n \t/*\n@@ -394,7 +393,6 @@ static void make_room_for_directories_of_df_conflicts(struct merge_options *o,\n \t\tif (S_ISREG(e->stages[2].mode) || S_ISLNK(e->stages[2].mode)) {\n \t\t\tlast_file = path;\n \t\t\tlast_len = len;\n-\t\t\tlast_e = e;\n \t\t} else {\n \t\t\tlast_file = NULL;\n \t\t}\n@@ -969,7 +967,6 @@ static int process_renames(struct merge_options *o,\n \t}\n \n \tfor (i = 0, j = 0; i < a_renames->nr || j < b_renames->nr;) {\n-\t\tchar *src;\n \t\tstruct string_list *renames1, *renames2Dst;\n \t\tstruct rename *ren1 = NULL, *ren2 = NULL;\n \t\tconst char *branch1, *branch2;\n@@ -1004,7 +1001,6 @@ static int process_renames(struct merge_options *o,\n \t\t\tren2 = ren1;\n \t\t\tren1 = tmp;\n \t\t}\n-\t\tsrc = ren1->pair->one->path;\n \n \t\tren1->dst_entry->processed = 1;\n \t\tren1->src_entry->processed = 1;\ndiff --git a/reachable.c b/reachable.c\nindex a03fabf..3fc6b1d 100644\n--- a/reachable.c\n+++ b/reachable.c\n@@ -70,16 +70,11 @@ static void process_tree(struct tree *tree,\n static void process_tag(struct tag *tag, struct object_array *p, const char *name)\n {\n \tstruct object *obj = &tag->object;\n-\tstruct name_path me;\n \n \tif (obj->flags & SEEN)\n \t\treturn;\n \tobj->flags |= SEEN;\n \n-\tme.up = NULL;\n-\tme.elem = \"tag:/\";\n-\tme.elem_len = 5;\n-\n \tif (parse_tag(tag) < 0)\n \t\tdie(\"bad tag object %s\", sha1_to_hex(obj->sha1));\n \tif (tag->tagged)\ndiff --git a/test-subprocess.c b/test-subprocess.c\nindex 667d3e5..8926bc5 100644\n--- a/test-subprocess.c\n+++ b/test-subprocess.c\n@@ -3,11 +3,10 @@\n \n int main(int argc, char **argv)\n {\n-\tconst char *prefix;\n \tstruct child_process cp;\n \tint nogit = 0;\n \n-\tprefix = setup_git_directory_gently(&nogit);\n+\tsetup_git_directory_gently(&nogit);\n \tif (nogit)\n \t\tdie(\"No git repo found\");\n \tif (!strcmp(argv[1], \"--setup-work-tree\")) {\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 0c5b1bd..a4f40fb 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -561,10 +561,9 @@ static int push_refs_with_push(struct transport *transport,\n \tint mirror = flags & TRANSPORT_PUSH_MIRROR;\n \tstruct helper_data *data = transport->data;\n \tstruct strbuf buf = STRBUF_INIT;\n-\tstruct child_process *helper;\n \tstruct ref *ref;\n \n-\thelper = get_helper(transport);\n+\tget_helper(transport);\n \tif (!data->push)\n \t\treturn 1;\n \n-- \n1.7.4.msysgit.0.167.ge91fd\n"},{"id":"164043","messageId":"91afab0d46939238dd7c90ea2902b02cae083316.1300798069.git.johannes.schindelin@gmx.de","threadId":"26826","inReplyTo":"cover.1300798069.git.johannes.schindelin@gmx.de","subject":"[PATCH 2/2] Actually use retval","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2011-03-22T12:50:46Z","receivedAt":"2011-03-22T12:50:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This is most likely a bug. Nocited by gcc 4.6.0.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tAlternatively, retval's declaration and assignment can go.\n\n tree-diff.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/tree-diff.c b/tree-diff.c\nindex 3954281..9e2dc70 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -58,7 +58,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2,\n \t\topt->change(opt, mode1, mode2, sha1, sha2, base->buf, 0, 0);\n \t}\n \tstrbuf_setlen(base, old_baselen);\n-\treturn 0;\n+\treturn retval;\n }\n \n /* A whole sub-tree went away or appeared */\n-- \n1.7.4.msysgit.0.167.ge91fd\n"},{"id":"164069","messageId":"7v7hbrkovz.fsf@alter.siamese.dyndns.org","threadId":"26826","inReplyTo":"91afab0d46939238dd7c90ea2902b02cae083316.1300798069.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 2/2] Actually use retval","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-22T18:10:08Z","receivedAt":"2011-03-22T18:10:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n\n> This is most likely a bug. Nocited by gcc 4.6.0.\n\nWhile I don't doubt gcc 4.6.0 found the retval assigned is not used, I\nthink you misunderstood the value returned from this function.\n\nThe caller uses the return value to decide if an entry from t1 (and not\nfrom t2) was consumed, if an entry each from both t1 and t2 were consumed,\nor an entry from t2 (and not from t1) was consumed.  It doesn't change the\nfact that the entry at the beginning of each tree we looked at in this\nfunction at that point shared the same name and we consumed them, whatever\nthe call to diff_tree_sha1() to run a recursive comparison between the\ntrees found.\n\nThe likely fix would be to remove assignment to retval instead.\n"},{"id":"164070","messageId":"7v39mfkons.fsf@alter.siamese.dyndns.org","threadId":"26826","inReplyTo":"2aeb4f915afaf712c1d5bba46f4010a325ea14c5.1300798069.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 1/2] Remove unused variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-22T18:15:03Z","receivedAt":"2011-03-22T18:15:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n\n> Noticed by gcc 4.6.0.\n>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n\nThanks.\n\nI'll cherry pick only some bits, not because the changes in this patch are\nwrong, but because I know some areas are in flux and I don't want to deal\nwith merge conflicts of the \"one side fixed a small bug, the other side\nmajorly rewrote it and made the bug irrelevant\" kind.\n"},{"id":"164071","messageId":"alpine.DEB.1.00.1103221918540.1561@bonsai2","threadId":"26826","inReplyTo":"7v7hbrkovz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] Actually use retval","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2011-03-22T18:20:14Z","receivedAt":"2011-03-22T18:20:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 22 Mar 2011, Junio C Hamano wrote:\n\n> Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n> \n> > This is most likely a bug. Nocited by gcc 4.6.0.\n> \n> While I don't doubt gcc 4.6.0 found the retval assigned is not used, I \n> think you misunderstood the value returned from this function.\n\nNope. I did not misunderstand. I did not understand in the first place. \nThat is what I described pretty explicitly in the cover letter.\n\n> The caller uses the return value to decide if an entry from t1 (and not \n> from t2) was consumed, if an entry each from both t1 and t2 were \n> consumed, or an entry from t2 (and not from t1) was consumed.  It \n> doesn't change the fact that the entry at the beginning of each tree we \n> looked at in this function at that point shared the same name and we \n> consumed them, whatever the call to diff_tree_sha1() to run a recursive \n> comparison between the trees found.\n> \n> The likely fix would be to remove assignment to retval instead.\n\nThanks, that is the alternative I suggested after the dashdashdash.\n\nCiao,\nJohannes\n"},{"id":"164073","messageId":"alpine.DEB.1.00.1103221940050.1561@bonsai2","threadId":"26826","inReplyTo":"7v39mfkons.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] Remove unused variables","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2011-03-22T18:42:06Z","receivedAt":"2011-03-22T18:42:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 22 Mar 2011, Junio C Hamano wrote:\n\n> Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n> \n> > Noticed by gcc 4.6.0.\n> >\n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > ---\n> \n> Thanks.\n> \n> I'll cherry pick only some bits, not because the changes in this patch \n> are wrong, but because I know some areas are in flux and I don't want to \n> deal with merge conflicts of the \"one side fixed a small bug, the other \n> side majorly rewrote it and made the bug irrelevant\" kind.\n\nUnderstandable. I will have to deal with the merge conflicts, though, as I \nam stuck with 4.6.0 (and self-imposed -Werror, as you might remember) for \nthe 64-bit Windows build. So I will keep the remains of the patch after \nrebasing in our 'devel' branch.\n\nCiao,\nJohannes\n"},{"id":"164083","messageId":"20110322194309.GA9677@elie","threadId":"26826","inReplyTo":"2aeb4f915afaf712c1d5bba46f4010a325ea14c5.1300798069.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 1/2] Remove unused variables","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-22T19:43:09Z","receivedAt":"2011-03-22T19:43:09Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nThese all look good.  Some notes:\n\nJohannes Schindelin wrote:\n\n> --- a/builtin/fsck.c\n> +++ b/builtin/fsck.c\n> @@ -140,11 +140,10 @@ static int traverse_reachable(void)\n>  \tint result = 0;\n>  \twhile (pending.nr) {\n>  \t\tstruct object_array_entry *entry;\n> -\t\tstruct object *obj, *parent;\n> +\t\tstruct object *obj;\n\nCleaning up after v1.7.4.1~1^2~1 (fsck: drop unused parameter from\ntraverse_one_object(), 2011-01-26).  Sensible.\n\n> --- a/builtin/remote-ext.c\n> +++ b/builtin/remote-ext.c\n> @@ -30,16 +30,12 @@ static char *strip_escapes(const char *str, const char *service,\n>  \tsize_t rpos = 0;\n>  \tint escape = 0;\n>  \tchar special = 0;\n> -\tsize_t pslen = 0;\n> -\tsize_t pSlen = 0;\n\nThese were never used; sorry to have missed it before.\n\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -1242,7 +1242,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n>  \tuintmax_t max_change = 0, max_len = 0;\n>  \tint total_files = data->nr;\n>  \tint width, name_width;\n> -\tconst char *reset, *set, *add_c, *del_c;\n> +\tconst char *reset, *add_c, *del_c;\n\nAfter v1.6.4-rc0~159^2 (diff: do not color --stat output like patch\ncontext, 2009-04-25), the \"patch context\" color is not used in\ndiffstats.\n\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -357,7 +357,6 @@ static void make_room_for_directories_of_df_conflicts(struct merge_options *o,\n>  \t */\n>  \tconst char *last_file = NULL;\n>  \tint last_len = 0;\n> -\tstruct stage_data *last_e;\n\nYep, was never used (and while the code to set it is correct, it\nis better to remove it than to let it bitrot).\n\n> @@ -969,7 +967,6 @@ static int process_renames(struct merge_options *o,\n>  \t}\n>  \n>  \tfor (i = 0, j = 0; i < a_renames->nr || j < b_renames->nr;) {\n> -\t\tchar *src;\n\nLikewise.\n\n> --- a/reachable.c\n> +++ b/reachable.c\n> @@ -70,16 +70,11 @@ static void process_tree(struct tree *tree,\n>  static void process_tag(struct tag *tag, struct object_array *p, const char *name)\n>  {\n>  \tstruct object *obj = &tag->object;\n> -\tstruct name_path me;\n>  \n>  \tif (obj->flags & SEEN)\n>  \t\treturn;\n>  \tobj->flags |= SEEN;\n>  \n> -\tme.up = NULL;\n> -\tme.elem = \"tag:/\";\n> -\tme.elem_len = 5;\n> -\n>  \tif (parse_tag(tag) < 0)\n>  \t\tdie(\"bad tag object %s\", sha1_to_hex(obj->sha1));\n>  \tif (tag->tagged)\n\nDoes this code make sense at all?  It peels a tag once in order to\nmark the object it refers to with \"add_object\", which adds the object\nto the array referred to by \"p\", that nobody actually uses.  Maybe\nthis is a remnant of an earlier implementation for \"git log\n--objects\"?\n\nPeeling of tags is already taken care of by revision::handle_commit,\nso I think the function could be stripped down to\n\n static void process_tag(struct tag *tag, struct object_array *p, const char *name)\n {\n\tstruct object *obj = &tag->object;\n \n\tif (obj->flags & SEEN)\n\t\treturn;\n\tobj->flags |= SEEN;\n }\n\nwhich seems to work okay still.\n\n> --- a/test-subprocess.c\n> +++ b/test-subprocess.c\n> @@ -3,11 +3,10 @@\n[...]\n> -\tprefix = setup_git_directory_gently(&nogit);\n> +\tsetup_git_directory_gently(&nogit);\n\nIt might be more idiomatic to use setup_git_directory, but this is\nbetter because conservative.\n\n> --- a/transport-helper.c\n> +++ b/transport-helper.c\n> @@ -561,10 +561,9 @@ static int push_refs_with_push(struct transport *transport,\n>  \tint mirror = flags & TRANSPORT_PUSH_MIRROR;\n>  \tstruct helper_data *data = transport->data;\n>  \tstruct strbuf buf = STRBUF_INIT;\n> -\tstruct child_process *helper;\n\nCleaning up after v1.7.0-rc0~62^2~12 (Add remote helper debug mode,\n2009-12-09).  The remote helper process is accessed through\ntransport->data->helper.\n\nThanks and hope that helps.\nJonathan\n"},{"id":"164105","messageId":"20110323012654.GA10621@elie","threadId":"26826","inReplyTo":"7v7hbrkovz.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/2] Re: [PATCH 2/2] Actually use retval","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-23T01:26:54Z","receivedAt":"2011-03-23T01:26:54Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> The likely fix would be to remove assignment to retval instead.\n\nLike this, maybe?\n\nJonathan Nieder (2):\n  tree-diff: make diff_tree() return void\n  tree-diff: make diff_tree_sha1() return void\n\n diff.h      |    4 ++--\n revision.c  |    9 +++------\n tree-diff.c |   17 ++++++-----------\n 3 files changed, 11 insertions(+), 19 deletions(-)\n"},{"id":"164106","messageId":"20110323012844.GB10621@elie","threadId":"26826","inReplyTo":"20110323012654.GA10621@elie","subject":"[PATCH 1/2] tree-diff: make diff_tree return void","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-23T01:28:44Z","receivedAt":"2011-03-23T01:28:44Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"diff_tree (which runs a diff between two trees) always returns 0 and\ndies on errors.  Some day it may change to change it to return -1 on\nerror but it will be easy to adjust callers then and until then the\nreturn value is just confusing.\n\nThis way at least callers are consistent in ignoring the return value\nand new readers don't have to wonder if diff_tree returns its diff\nresult like \"diff --exit-code\" would.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n diff.h      |    2 +-\n revision.c  |    5 ++---\n tree-diff.c |   13 +++++--------\n 3 files changed, 8 insertions(+), 12 deletions(-)\n\ndiff --git a/diff.h b/diff.h\nindex 007a055..4fde7f3 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -163,7 +163,7 @@ extern const char mime_boundary_leader[];\n \n extern void diff_tree_setup_paths(const char **paths, struct diff_options *);\n extern void diff_tree_release_paths(struct diff_options *);\n-extern int diff_tree(struct tree_desc *t1, struct tree_desc *t2,\n+extern void diff_tree(struct tree_desc *t1, struct tree_desc *t2,\n \t\t     const char *base, struct diff_options *opt);\n extern int diff_tree_sha1(const unsigned char *old, const unsigned char *new,\n \t\t\t  const char *base, struct diff_options *opt);\ndiff --git a/revision.c b/revision.c\nindex 86d2470..a6e78c9 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -337,7 +337,6 @@ static int rev_compare_tree(struct rev_info *revs, struct commit *parent, struct\n \n static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit)\n {\n-\tint retval;\n \tvoid *tree;\n \tunsigned long size;\n \tstruct tree_desc empty, real;\n@@ -354,10 +353,10 @@ static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit)\n \n \ttree_difference = REV_TREE_SAME;\n \tDIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);\n-\tretval = diff_tree(&empty, &real, \"\", &revs->pruning);\n+\tdiff_tree(&empty, &real, \"\", &revs->pruning);\n \tfree(tree);\n \n-\treturn retval >= 0 && (tree_difference == REV_TREE_SAME);\n+\treturn tree_difference == REV_TREE_SAME;\n }\n \n static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)\ndiff --git a/tree-diff.c b/tree-diff.c\nindex 3954281..d1a7ae9 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -138,7 +138,7 @@ static void skip_uninteresting(struct tree_desc *t, struct strbuf *base,\n \t}\n }\n \n-int diff_tree(struct tree_desc *t1, struct tree_desc *t2,\n+void diff_tree(struct tree_desc *t1, struct tree_desc *t2,\n \t      const char *base_str, struct diff_options *opt)\n {\n \tstruct strbuf base;\n@@ -190,7 +190,6 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,\n \t}\n \n \tstrbuf_release(&base);\n-\treturn 0;\n }\n \n /*\n@@ -285,7 +284,6 @@ int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const cha\n \tvoid *tree1, *tree2;\n \tstruct tree_desc t1, t2;\n \tunsigned long size1, size2;\n-\tint retval;\n \n \ttree1 = read_object_with_reference(old, tree_type, &size1, NULL);\n \tif (!tree1)\n@@ -295,7 +293,7 @@ int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const cha\n \t\tdie(\"unable to read destination tree (%s)\", sha1_to_hex(new));\n \tinit_tree_desc(&t1, tree1, size1);\n \tinit_tree_desc(&t2, tree2, size2);\n-\tretval = diff_tree(&t1, &t2, base, opt);\n+\tdiff_tree(&t1, &t2, base, opt);\n \tif (!*base && DIFF_OPT_TST(opt, FOLLOW_RENAMES) && diff_might_be_rename()) {\n \t\tinit_tree_desc(&t1, tree1, size1);\n \t\tinit_tree_desc(&t2, tree2, size2);\n@@ -303,12 +301,11 @@ int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const cha\n \t}\n \tfree(tree1);\n \tfree(tree2);\n-\treturn retval;\n+\treturn 0;\n }\n \n int diff_root_tree_sha1(const unsigned char *new, const char *base, struct diff_options *opt)\n {\n-\tint retval;\n \tvoid *tree;\n \tunsigned long size;\n \tstruct tree_desc empty, real;\n@@ -319,9 +316,9 @@ int diff_root_tree_sha1(const unsigned char *new, const char *base, struct diff_\n \tinit_tree_desc(&real, tree, size);\n \n \tinit_tree_desc(&empty, \"\", 0);\n-\tretval = diff_tree(&empty, &real, base, opt);\n+\tdiff_tree(&empty, &real, base, opt);\n \tfree(tree);\n-\treturn retval;\n+\treturn 0;\n }\n \n void diff_tree_release_paths(struct diff_options *opt)\n-- \n1.7.4.1\n"},{"id":"164107","messageId":"20110323013021.GC10621@elie","threadId":"26826","inReplyTo":"20110323012654.GA10621@elie","subject":"[PATCH 2/2] tree-diff: make diff_tree_sha1 return void","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-23T01:30:21Z","receivedAt":"2011-03-23T01:30:21Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"diff_tree_sha1 does not return -1 on errors; it die()s.  Now it would\nobviously be lovely to change that so long-running programs could call\ndiff_tree_sha1 without fear, but until then, let's face the reality:\n\n - nobody has tested what would happen if it started returning\n   instead of dying on failure;\n - the callers overwhelmingly ignore the return value.\n\nReturning void also saves readers the trouble of checking\ndocumentation each time they use diff_tree_sha1 to see if its return\nvalue follows the GNU diff exit code convention.\n\nSo it's just more honest to return void.  Do so.\n\nNoticed with gcc-4.6 -Wunused-but-set-variable (because one caller\ncouldn't decide --- it saves the return value, but never does anything\nwith it).\n\nNoticed-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n diff.h      |    2 +-\n revision.c  |    4 +---\n tree-diff.c |    6 ++----\n 3 files changed, 4 insertions(+), 8 deletions(-)\n\ndiff --git a/diff.h b/diff.h\nindex 4fde7f3..0a8a06c 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -165,7 +165,7 @@ extern void diff_tree_setup_paths(const char **paths, struct diff_options *);\n extern void diff_tree_release_paths(struct diff_options *);\n extern void diff_tree(struct tree_desc *t1, struct tree_desc *t2,\n \t\t     const char *base, struct diff_options *opt);\n-extern int diff_tree_sha1(const unsigned char *old, const unsigned char *new,\n+extern void diff_tree_sha1(const unsigned char *old, const unsigned char *new,\n \t\t\t  const char *base, struct diff_options *opt);\n extern int diff_root_tree_sha1(const unsigned char *new, const char *base,\n                                struct diff_options *opt);\ndiff --git a/revision.c b/revision.c\nindex a6e78c9..0c810b4 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -329,9 +329,7 @@ static int rev_compare_tree(struct rev_info *revs, struct commit *parent, struct\n \n \ttree_difference = REV_TREE_SAME;\n \tDIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);\n-\tif (diff_tree_sha1(t1->object.sha1, t2->object.sha1, \"\",\n-\t\t\t   &revs->pruning) < 0)\n-\t\treturn REV_TREE_DIFFERENT;\n+\tdiff_tree_sha1(t1->object.sha1, t2->object.sha1, \"\", &revs->pruning);\n \treturn tree_difference;\n }\n \ndiff --git a/tree-diff.c b/tree-diff.c\nindex d1a7ae9..03b8ca0 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -17,7 +17,6 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2,\n \tconst unsigned char *sha1, *sha2;\n \tint cmp, pathlen1, pathlen2;\n \tint old_baselen = base->len;\n-\tint retval = 0;\n \n \tsha1 = tree_entry_extract(t1, &path1, &mode1);\n \tsha2 = tree_entry_extract(t2, &path2, &mode2);\n@@ -53,7 +52,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2,\n \t\t\t\t    sha1, sha2, base->buf, 0, 0);\n \t\t}\n \t\tstrbuf_addch(base, '/');\n-\t\tretval = diff_tree_sha1(sha1, sha2, base->buf, opt);\n+\t\tdiff_tree_sha1(sha1, sha2, base->buf, opt);\n \t} else {\n \t\topt->change(opt, mode1, mode2, sha1, sha2, base->buf, 0, 0);\n \t}\n@@ -279,7 +278,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co\n \tq->nr = 1;\n }\n \n-int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const char *base, struct diff_options *opt)\n+void diff_tree_sha1(const unsigned char *old, const unsigned char *new, const char *base, struct diff_options *opt)\n {\n \tvoid *tree1, *tree2;\n \tstruct tree_desc t1, t2;\n@@ -301,7 +300,6 @@ int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const cha\n \t}\n \tfree(tree1);\n \tfree(tree2);\n-\treturn 0;\n }\n \n int diff_root_tree_sha1(const unsigned char *new, const char *base, struct diff_options *opt)\n-- \n1.7.4.1\n"}]}