Hi Miriam,
On Mon, 20 Jan 2020, Miriam Rubio wrote:
> From: Pranit Bauva <pranit.bauva@xxxxxxxxx>
>
> Since we want to get rid of git-bisect.sh it would be necessary to
> convert those exit() calls to return statements so that errors can be
> reported.
>
> Emulate try catch in C by converting `exit(<positive-value>)` to
> `return <negative-value>`. Follow POSIX conventions to return
> <negative-value> to indicate error.
>
> Turn `exit()` to `return` calls in `bisect_next_all()`.
>
> All the functions calling `bisect_next_all()` are already able to
> handle return values from it.
So this patch brings more magic values that really would like to become
`enum` values. At this point, we're talking about `bisect_next_all()`
which is _not_ a file-local (`static`) function. Therefore, we now really
wade into API territory where an `enum` is no longer just a nice thing,
but a necessary thing: `bisect_next_all()` is a function that can be
called from other source files.
As far as I can see, this patch concludes the "libify" part of the patch
series (read: it would have been the second patch series I would have
suggested to split out from an otherwise too-long-to-be-reviewed set of
patches, if I had been your mentor), and I'll leave it at that for
tonight; Hopefully I will find some time to review more of these patches
tomorrow.
Ciao,
Johannes
>
> Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
> Mentored-by: Johannes Schindelin <Johannes.Schindelin@xxxxxx>
> Signed-off-by: Pranit Bauva <pranit.bauva@xxxxxxxxx>
> Signed-off-by: Tanushree Tumane <tanushreetumane@xxxxxxxxx>
> Signed-off-by: Miriam Rubio <mirucam@xxxxxxxxx>
> ---
> bisect.c | 40 +++++++++++++++++++++++++++++-----------
> 1 file changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/bisect.c b/bisect.c
> index acb5a13911..33f2829c19 100644
> --- a/bisect.c
> +++ b/bisect.c
> @@ -967,10 +967,10 @@ void read_bisect_terms(const char **read_bad, const char **read_good)
> }
>
> /*
> - * We use the convention that exiting with an exit code 10 means that
> - * the bisection process finished successfully.
> - * In this case the calling shell script should exit 0.
> - *
> + * We use the convention that return -10 means the bisection process
> + * finished successfully.
> + * In this case the calling function or command should not turn a -10
> + * return code into an error or a non zero exit code.
> * If no_checkout is non-zero, the bisection process does not
> * checkout the trial commit but instead simply updates BISECT_HEAD.
> */
> @@ -1000,23 +1000,35 @@ int bisect_next_all(struct repository *r, const char *prefix, int no_checkout)
>
> if (!revs.commits) {
> /*
> - * We should exit here only if the "bad"
> + * We should return error here only if the "bad"
> * commit is also a "skip" commit.
> */
> res = error_if_skipped_commits(tried, NULL);
> if (res)
> - exit(-res);
> + return res;
> printf(_("%s was both %s and %s\n"),
> oid_to_hex(current_bad_oid),
> term_good,
> term_bad);
> - exit(1);
> +
> + /*
> + * We don't want to clean the bisection state
> + * as we need to get back to where we started
> + * by using `git bisect reset`.
> + */
> + return -1;
> }
>
> if (!all) {
> fprintf(stderr, _("No testable commit found.\n"
> "Maybe you started with bad path parameters?\n"));
> - exit(4);
> +
> + /*
> + * We don't want to clean the bisection state
> + * as we need to get back to where we started
> + * by using `git bisect reset`.
> + */
> + return -4;
> }
>
> bisect_rev = &revs.commits->item->object.oid;
> @@ -1024,12 +1036,18 @@ int bisect_next_all(struct repository *r, const char *prefix, int no_checkout)
> if (oideq(bisect_rev, current_bad_oid)) {
> res = error_if_skipped_commits(tried, current_bad_oid);
> if (res)
> - exit(-res);
> + return res;
> printf("%s is the first %s commit\n", oid_to_hex(bisect_rev),
> term_bad);
> +
> show_diff_tree(r, prefix, revs.commits->item);
> - /* This means the bisection process succeeded. */
> - exit(10);
> + /*
> + * This means the bisection process succeeded.
> + * Using -10 so that the call chain can simply check
> + * for negative return values for early returns up
> + * until the cmd_bisect__helper() caller.
> + */
> + return -10;
> }
>
> nr = all - reaches - 1;
> --
> 2.21.1 (Apple Git-122.3)
>
>
