Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > Change various cmd_* functions that claim no return an "int" to use s/no return/to return/ > "return" instead of exit() to indicate an exit code. These were not > marked with NORETURN, Up to this point, it is well written. > 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. But I think this is a hyperbole. File descritors are closed when we exit without git.c's help, thank-you-very-much ;-), and if we do have clean-ups that are truly important, we would have arranged them to happen in the atexit handler, so it is not a crime for functions called from the subcommand dispatchers to exit themselves (as long as they exit sensibly, e.g. without doing nonsense like exit(-1)). It nevertheless is a good idea because it encourages good code hygiene, just like marking with NORETURN if the function must exit. Selling this change as if it were a correctness fix (i.e. we were exiting and missed these important clean-ups that the caller wanted to do after we return) is misleading. > 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@xxxxxxxxx> > --- > > 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. The range diff looks good to me. Thanks.
