Ramkumar Ramachandra <artagnon@xxxxxxxxx> writes:
> +static int error_dirty_index(const char *me)
> {
> + if (read_cache_unmerged())
> + return error_resolve_conflict(me);
> +
> + int ret = error(_("Your local changes would be overwritten by %s.\n"), me);
> + if (advice_commit_before_merge)
> + advise(_("Please, commit your changes or stash them to proceed."));
> + return ret;
> }
I like this rewrite whose result is short-and-sweet, but you do not even
need the "ret" variable. error() always yields -1, no?
> @@ -594,14 +584,28 @@ static int revert_or_cherry_pick(int argc, const char **argv)
>
> int cmd_revert(int argc, const char **argv, const char *prefix)
> {
> + int res = 0;
> if (isatty(0))
> edit = 1;
> action = REVERT;
> - return revert_or_cherry_pick(argc, argv);
> + res = revert_or_cherry_pick(argc, argv);
> + if (res > 0)
> + /* Exit status from conflict */
> + return res;
> + if (res < 0)
> + /* Other error */
> + exit(128);
> + return 0;
> }
>
> int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
> {
> + int res = 0;
> action = CHERRY_PICK;
> - return revert_or_cherry_pick(argc, argv);
> + res = revert_or_cherry_pick(argc, argv);
> + if (res > 0)
> + return res;
> + if (res < 0)
> + exit(128);
> + return 0;
> }
This hunk is dubious.
- Why initialize res to zero if it always is assigned the return value of
revert_or_cherry_pick() before it is used?
- The called function seems to return errors from various places but as
far as I see they are all return value of error(), so it would be
equivalent to
if (r_o_c_p(...))
exit(128);
return 0;
If you are going to introduce different return values from r-o-c-p() in a
later patch, these functions should be updated in that patch, I think.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html