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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.