[PATCH v2] builtins + test helpers: use return instead of exit() in cmd_*

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

Change various cmd_* functions that claim no return an "int" to use
"return" instead of exit() to indicate an exit code. These were not
marked with NORETURN, and by directly exit()-ing we'll skip the
cleanup git.c would otherwise do (e.g. closing fd's, erroring if we
can't). See run_builtin() in git.c.

In the case of shell.c and sh-i18n--envsubst.c this was the result of
an incomplete migration to using a cmd_main() in 3f2e2297b9 (add an
extra level of indirection to main(), 2016-07-01).

This was spotted by SunCC 12.5 on Solaris 10 (gcc210 on the gccfarm).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

Clarified the commit message, and made the same s/exit/return/g change
in shell.c and sh-i18n--envsubst.c. I also missed an "exit(2)" in a
brach in builtin/merge-ours.c.

Range-diff against v1:
1:  61d7e6e079 ! 1:  f225b78e01 builtins + test helpers: use return instead of exit() in cmd_*
    @@ Metadata
      ## Commit message ##
         builtins + test helpers: use return instead of exit() in cmd_*
     
    -    Change various cmd_* functions to use "return" instead of exit() to
    -    indicate an exit code. On Solaris with SunCC the compiler legitimately
    -    complains about these, since we'll e.g. skip the cleanup (e.g. closing
    -    fd's, erroring if we can't) in git.c's run_builtin() when we exit()
    -    directly like this.
    +    Change various cmd_* functions that claim no return an "int" to use
    +    "return" instead of exit() to indicate an exit code. These were not
    +    marked with NORETURN, and by directly exit()-ing we'll skip the
    +    cleanup git.c would otherwise do (e.g. closing fd's, erroring if we
    +    can't). See run_builtin() in git.c.
    +
    +    In the case of shell.c and sh-i18n--envsubst.c this was the result of
    +    an incomplete migration to using a cmd_main() in 3f2e2297b9 (add an
    +    extra level of indirection to main(), 2016-07-01).
    +
    +    This was spotted by SunCC 12.5 on Solaris 10 (gcc210 on the gccfarm).
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    @@ builtin/difftool.c: static int run_file_diff(int prompt, const char *prefix,
     
      ## builtin/merge-ours.c ##
     @@ builtin/merge-ours.c: int cmd_merge_ours(int argc, const char **argv, const char *prefix)
    + 	if (read_cache() < 0)
      		die_errno("read_cache failed");
      	if (index_differs_from(the_repository, "HEAD", NULL, 0))
    - 		exit(2);
    +-		exit(2);
     -	exit(0);
    ++		return 2;
     +	return 0;
      }
     
    @@ builtin/mktree.c: int cmd_mktree(int ac, const char **av, const char *prefix)
     +	return 0;
      }
     
    + ## sh-i18n--envsubst.c ##
    +@@ sh-i18n--envsubst.c: cmd_main (int argc, const char *argv[])
    +   if (ferror (stderr) || fflush (stderr))
    +     {
    +       fclose (stderr);
    +-      exit (EXIT_FAILURE);
    ++      return (EXIT_FAILURE);
    +     }
    +   if (fclose (stderr) && errno != EBADF)
    +-    exit (EXIT_FAILURE);
    ++    return (EXIT_FAILURE);
    + 
    +-  exit (EXIT_SUCCESS);
    ++  return (EXIT_SUCCESS);
    + }
    + 
    + /* Parse the string and invoke the callback each time a $VARIABLE or
    +
    + ## shell.c ##
    +@@ shell.c: int cmd_main(int argc, const char **argv)
    + 		default:
    + 			continue;
    + 		}
    +-		exit(cmd->exec(cmd->name, arg));
    ++		return cmd->exec(cmd->name, arg);
    + 	}
    + 
    + 	cd_to_homedir();
    +
      ## t/helper/test-hash-speed.c ##
     @@ t/helper/test-hash-speed.c: int cmd__hash_speed(int ac, const char **av)
      		free(p);

 builtin/difftool.c          | 5 ++---
 builtin/merge-ours.c        | 4 ++--
 builtin/mktree.c            | 2 +-
 sh-i18n--envsubst.c         | 6 +++---
 shell.c                     | 2 +-
 t/helper/test-hash-speed.c  | 2 +-
 t/helper/test-hash.c        | 2 +-
 t/helper/test-match-trees.c | 2 +-
 t/helper/test-reach.c       | 2 +-
 9 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/builtin/difftool.c b/builtin/difftool.c
index 89334b77fb..6a9242a803 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -675,7 +675,7 @@ static int run_file_diff(int prompt, const char *prefix,
 		"GIT_PAGER=", "GIT_EXTERNAL_DIFF=git-difftool--helper", NULL,
 		NULL
 	};
-	int ret = 0, i;
+	int i;
 
 	if (prompt > 0)
 		env[2] = "GIT_DIFFTOOL_PROMPT=true";
@@ -686,8 +686,7 @@ static int run_file_diff(int prompt, const char *prefix,
 	strvec_push(&args, "diff");
 	for (i = 0; i < argc; i++)
 		strvec_push(&args, argv[i]);
-	ret = run_command_v_opt_cd_env(args.v, RUN_GIT_CMD, prefix, env);
-	exit(ret);
+	return run_command_v_opt_cd_env(args.v, RUN_GIT_CMD, prefix, env);
 }
 
 int cmd_difftool(int argc, const char **argv, const char *prefix)
diff --git a/builtin/merge-ours.c b/builtin/merge-ours.c
index 4594507420..3583cff71c 100644
--- a/builtin/merge-ours.c
+++ b/builtin/merge-ours.c
@@ -28,6 +28,6 @@ int cmd_merge_ours(int argc, const char **argv, const char *prefix)
 	if (read_cache() < 0)
 		die_errno("read_cache failed");
 	if (index_differs_from(the_repository, "HEAD", NULL, 0))
-		exit(2);
-	exit(0);
+		return 2;
+	return 0;
 }
diff --git a/builtin/mktree.c b/builtin/mktree.c
index 891991b00d..ae78ca1c02 100644
--- a/builtin/mktree.c
+++ b/builtin/mktree.c
@@ -189,5 +189,5 @@ int cmd_mktree(int ac, const char **av, const char *prefix)
 		used=0; /* reset tree entry buffer for re-use in batch mode */
 	}
 	strbuf_release(&sb);
-	exit(0);
+	return 0;
 }
diff --git a/sh-i18n--envsubst.c b/sh-i18n--envsubst.c
index e7430b9aa8..6cd307ac2c 100644
--- a/sh-i18n--envsubst.c
+++ b/sh-i18n--envsubst.c
@@ -104,12 +104,12 @@ cmd_main (int argc, const char *argv[])
   if (ferror (stderr) || fflush (stderr))
     {
       fclose (stderr);
-      exit (EXIT_FAILURE);
+      return (EXIT_FAILURE);
     }
   if (fclose (stderr) && errno != EBADF)
-    exit (EXIT_FAILURE);
+    return (EXIT_FAILURE);
 
-  exit (EXIT_SUCCESS);
+  return (EXIT_SUCCESS);
 }
 
 /* Parse the string and invoke the callback each time a $VARIABLE or
diff --git a/shell.c b/shell.c
index cef7ffdc9e..811e13b9c9 100644
--- a/shell.c
+++ b/shell.c
@@ -177,7 +177,7 @@ int cmd_main(int argc, const char **argv)
 		default:
 			continue;
 		}
-		exit(cmd->exec(cmd->name, arg));
+		return cmd->exec(cmd->name, arg);
 	}
 
 	cd_to_homedir();
diff --git a/t/helper/test-hash-speed.c b/t/helper/test-hash-speed.c
index 432233c7f0..f40d9ad0c2 100644
--- a/t/helper/test-hash-speed.c
+++ b/t/helper/test-hash-speed.c
@@ -57,5 +57,5 @@ int cmd__hash_speed(int ac, const char **av)
 		free(p);
 	}
 
-	exit(0);
+	return 0;
 }
diff --git a/t/helper/test-hash.c b/t/helper/test-hash.c
index 0a31de66f3..261c545b9d 100644
--- a/t/helper/test-hash.c
+++ b/t/helper/test-hash.c
@@ -54,5 +54,5 @@ int cmd_hash_impl(int ac, const char **av, int algo)
 		fwrite(hash, 1, algop->rawsz, stdout);
 	else
 		puts(hash_to_hex_algop(hash, algop));
-	exit(0);
+	return 0;
 }
diff --git a/t/helper/test-match-trees.c b/t/helper/test-match-trees.c
index b9fd427571..4079fdee06 100644
--- a/t/helper/test-match-trees.c
+++ b/t/helper/test-match-trees.c
@@ -23,5 +23,5 @@ int cmd__match_trees(int ac, const char **av)
 	shift_tree(the_repository, &one->object.oid, &two->object.oid, &shifted, -1);
 	printf("shifted: %s\n", oid_to_hex(&shifted));
 
-	exit(0);
+	return 0;
 }
diff --git a/t/helper/test-reach.c b/t/helper/test-reach.c
index cda804ed79..2f65c7f6a5 100644
--- a/t/helper/test-reach.c
+++ b/t/helper/test-reach.c
@@ -166,5 +166,5 @@ int cmd__reach(int ac, const char **av)
 		print_sorted_commit_ids(list);
 	}
 
-	exit(0);
+	return 0;
 }
-- 
2.32.0.rc3.434.gd8aed1f08a7