diff mbox series

[pacman-dev,10/10] makepkg: Don't use parameterless return

Message ID a196f35e0a0d1395bb6864d7a4d87542f711d7d9.1527783895.git.jan.steffens@gmail.com
State Accepted, archived
Headers show
Series [pacman-dev,01/10] libmakepkg/util/option: Refactor checking to reduce code duplication | expand

Commit Message

Jan Alexander Steffens May 31, 2018, 4:24 p.m. UTC 
It's especially dangerous in trap handlers since the return value of the
function becomes the return value of the last command before the trap,
not the last command in the current function. This applies to any
function executed in a trap handler, nested functions included.

In one case, install_packages failed (via return 14), which was inside a
conditional that then ran exit 14, which triggered the EXIT handler,
which called clean_up, which called remove_deps, which had !RMDEPS and
thus returned. The return value of remove_deps became the return value
of install_packages, triggering the ERR handler, which (due to another
problem) was still the user function handler, which then printed a
misleading error message and overrode the exit code with 4.
---
 scripts/makepkg.sh.in | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
diff mbox series

Patch

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 3a3f4c30..968fab0d 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -133,7 +133,7 @@  clean_up() {
 
 	if (( INFAKEROOT )); then
 		# Don't clean up when leaving fakeroot, we're not done yet.
-		return
+		return 0
 	fi
 
 	if (( (EXIT_CODE == E_OK || EXIT_CODE == E_INSTALL_FAILED) && CLEANUP )); then
@@ -313,7 +313,7 @@  resolve_deps() {
 }
 
 remove_deps() {
-	(( ! RMDEPS )) && return
+	(( ! RMDEPS )) && return 0
 
 	# check for packages removed during dependency install (e.g. due to conflicts)
 	# removing all installed packages is risky in this case
@@ -512,7 +512,7 @@  find_libdepends() {
 
 	if (( sodepends == 0 )); then
 		(( ${#depends[@]} )) && printf '%s\n' "${depends[@]}"
-		return
+		return 0
 	fi
 
 	local libdeps filename soarch sofile soname soversion
@@ -714,7 +714,7 @@  list_package_files() {
 }
 
 create_package() {
-	(( NOARCHIVE )) && return
+	(( NOARCHIVE )) && return 0
 
 	if [[ ! -d $pkgdir ]]; then
 		error "$(gettext "Missing %s directory.")" "\$pkgdir/"
@@ -779,12 +779,12 @@  create_debug_package() {
 
 	# check if a debug package was requested
 	if ! check_option "debug" "y" || ! check_option "strip" "y"; then
-		return
+		return 0
 	fi
 
 	# check if we have any debug symbols to package
 	if dir_is_empty "$pkgdir/usr/lib/debug"; then
-		return
+		return 0
 	fi
 
 	unset groups depends optdepends provides conflicts replaces backup install changelog
@@ -868,7 +868,7 @@  create_srcpackage() {
 }
 
 install_package() {
-	(( ! INSTALL )) && return
+	(( ! INSTALL )) && return 0
 
 	if (( ! SPLITPKG )); then
 		msg "$(gettext "Installing package %s with %s...")" "$pkgname" "$PACMAN -U"