From: Jonathan Nieder <jrnieder@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>, git@vger.kernel.org
Subject: [PATCH 2/2] tree-diff: make diff_tree_sha1 return void
Date: Tue, 22 Mar 2011 20:30:21 -0500 [thread overview]
Message-ID: <20110323013021.GC10621@elie> (raw)
In-Reply-To: <20110323012654.GA10621@elie>
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
prev parent reply other threads:[~2011-03-23 1:30 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-03-22 12:49 [PATCH 0/2] Fix issues found by gcc 4.6.0 Johannes Schindelin
2011-03-22 12:50 ` [PATCH 1/2] Remove unused variables Johannes Schindelin
2011-03-22 18:15 ` Junio C Hamano
2011-03-22 18:42 ` Johannes Schindelin
2011-03-22 19:43 ` Jonathan Nieder
2011-03-22 12:50 ` [PATCH 2/2] Actually use retval Johannes Schindelin
2011-03-22 18:10 ` Junio C Hamano
2011-03-22 18:20 ` Johannes Schindelin
2011-03-23 1:26 ` [PATCH 0/2] " Jonathan Nieder
2011-03-23 1:28 ` [PATCH 1/2] tree-diff: make diff_tree return void Jonathan Nieder
2011-03-23 1:30 ` Jonathan Nieder [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110323013021.GC10621@elie \
--to=jrnieder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johannes.schindelin@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).