Re: [PATCH 2/2] prepare_revision_walk: Check for return value in all places
Stefan Beller stefanbel...@gmail.com writes: Even the documentation tells us: You should check if it returns any error (non-zero return code) and if it does not, you can start using get_revision() to do the iteration. In preparation for this commit, I grepped all occurrences of prepare_revision_walk and added error messages, when there were none. Thanks. One niggling thing is that I do not seem to find a way for the function to actually return an error without dying in it, but these are independently good changes. Signed-off-by: Stefan Beller stefanbel...@gmail.com --- builtin/branch.c | 4 +++- builtin/commit.c | 3 ++- remote.c | 3 ++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 0591b22..ced422b 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -653,7 +653,9 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru add_pending_object(ref_list.revs, (struct object *) filter, ); ref_list.revs.limited = 1; - prepare_revision_walk(ref_list.revs); + + if (prepare_revision_walk(ref_list.revs)) + die(_(revision walk setup failed)); if (verbose) ref_list.maxwidth = calc_maxwidth(ref_list); } diff --git a/builtin/commit.c b/builtin/commit.c index 7867768..bb84e1d 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1055,7 +1055,8 @@ static const char *find_author_by_nickname(const char *name) revs.mailmap = mailmap; read_mailmap(revs.mailmap, NULL); - prepare_revision_walk(revs); + if (prepare_revision_walk(revs)) + die(revision walk setup failed); commit = get_revision(revs); if (commit) { struct pretty_print_context ctx = {0}; diff --git a/remote.c b/remote.c index 894db09..112e4d5 100644 --- a/remote.c +++ b/remote.c @@ -1893,7 +1893,8 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs) init_revisions(revs, NULL); setup_revisions(rev_argc, rev_argv, revs, NULL); - prepare_revision_walk(revs); + if (prepare_revision_walk(revs)) + die(revision walk setup failed); /* ... and count the commits on each side. */ *num_ours = 0; -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] prepare_revision_walk: Check for return value in all places
On 11.08.2014 21:09, Junio C Hamano wrote: Stefan Beller stefanbel...@gmail.com writes: Even the documentation tells us: You should check if it returns any error (non-zero return code) and if it does not, you can start using get_revision() to do the iteration. In preparation for this commit, I grepped all occurrences of prepare_revision_walk and added error messages, when there were none. Thanks. One niggling thing is that I do not seem to find a way for the function to actually return an error without dying in it, but these are independently good changes. Signed-off-by: Stefan Beller stefanbel...@gmail.com --- builtin/branch.c | 4 +++- builtin/commit.c | 3 ++- remote.c | 3 ++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 0591b22..ced422b 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -653,7 +653,9 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru add_pending_object(ref_list.revs, (struct object *) filter, ); ref_list.revs.limited = 1; -prepare_revision_walk(ref_list.revs); + +if (prepare_revision_walk(ref_list.revs)) +die(_(revision walk setup failed)); if (verbose) ref_list.maxwidth = calc_maxwidth(ref_list); } diff --git a/builtin/commit.c b/builtin/commit.c index 7867768..bb84e1d 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1055,7 +1055,8 @@ static const char *find_author_by_nickname(const char *name) revs.mailmap = mailmap; read_mailmap(revs.mailmap, NULL); -prepare_revision_walk(revs); +if (prepare_revision_walk(revs)) +die(revision walk setup failed); Just reviewed the patch myself and here in commit.c we should have a localized version, right? Should I resend the patch with the localisation or could you just amend that? commit = get_revision(revs); if (commit) { struct pretty_print_context ctx = {0}; diff --git a/remote.c b/remote.c index 894db09..112e4d5 100644 --- a/remote.c +++ b/remote.c @@ -1893,7 +1893,8 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs) init_revisions(revs, NULL); setup_revisions(rev_argc, rev_argv, revs, NULL); -prepare_revision_walk(revs); +if (prepare_revision_walk(revs)) +die(revision walk setup failed); /* ... and count the commits on each side. */ *num_ours = 0; -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] prepare_revision_walk: Check for return value in all places
Even the documentation tells us: You should check if it returns any error (non-zero return code) and if it does not, you can start using get_revision() to do the iteration. In preparation for this commit, I grepped all occurrences of prepare_revision_walk and added error messages, when there were none. Signed-off-by: Stefan Beller stefanbel...@gmail.com --- builtin/branch.c | 4 +++- builtin/commit.c | 3 ++- remote.c | 3 ++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 0591b22..ced422b 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -653,7 +653,9 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru add_pending_object(ref_list.revs, (struct object *) filter, ); ref_list.revs.limited = 1; - prepare_revision_walk(ref_list.revs); + + if (prepare_revision_walk(ref_list.revs)) + die(_(revision walk setup failed)); if (verbose) ref_list.maxwidth = calc_maxwidth(ref_list); } diff --git a/builtin/commit.c b/builtin/commit.c index 7867768..bb84e1d 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1055,7 +1055,8 @@ static const char *find_author_by_nickname(const char *name) revs.mailmap = mailmap; read_mailmap(revs.mailmap, NULL); - prepare_revision_walk(revs); + if (prepare_revision_walk(revs)) + die(revision walk setup failed); commit = get_revision(revs); if (commit) { struct pretty_print_context ctx = {0}; diff --git a/remote.c b/remote.c index 894db09..112e4d5 100644 --- a/remote.c +++ b/remote.c @@ -1893,7 +1893,8 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs) init_revisions(revs, NULL); setup_revisions(rev_argc, rev_argv, revs, NULL); - prepare_revision_walk(revs); + if (prepare_revision_walk(revs)) + die(revision walk setup failed); /* ... and count the commits on each side. */ *num_ours = 0; -- 2.1.0.rc2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html