Refactor: trap_exit and cleanup_mountpoints: Simplify logic

* Eliminate duplicate logic by apply `cleanup_mountpoint` to `source_fs_mountpoint` and `target_fs_mountpoint` separately. * Drop `echo_with_color` in favor of `printf_with_color` * Use flags to eliminate duplicate messages Signed-off-by: 林博仁(Buo-ren Lin) <[email protected]>
slacka · May 8, 2018 · fe83365 · fe83365
1 parent 952e2da
commit fe83365
Showing 1 changed file with 74 additions and 59 deletions.
diff --git a/src/woeusb b/src/woeusb
@@ -1630,61 +1630,43 @@ install_legacy_pc_bootloader_grub_config(){
 	} > "${grub_cfg}"; unset target_fs_uuid
 }; declare -fr install_legacy_pc_bootloader_grub_config
 
-## Unmount mounted filesystems and clean-up mountpoints before exiting program
-## exit status:
-##     unclean(2): Not fully clean, target device can be safely detach from host
-##     unsafe(3): Target device cannot be safely detach from host
-cleanup_mountpoints(){
-	util_check_function_parameters_quantity 3 "${#}"
-	local -r source_fs_mountpoint="${1}"; shift
-	local -r target_fs_mountpoint="${1}"; shift
-	local -ir only_for_gui="${1}"
-
-	local clean_result='unknown'
-
-	# FIXME: Why `trigger_wxGenericProgressDialog_pulse off` here?
-	trigger_wxGenericProgressDialog_pulse\
-		off\
-		"${only_for_gui}"
+## Unmount mounted filesystem and clean-up mountpoint before exiting program
+## return_value: 1 - Failed to unmount / 2 - Failed to remove mountpoint
+cleanup_mountpoint(){
+	util_check_function_parameters_quantity 2 "${#}"
+	local -r fs_mountpoint="${1}"; shift
+	local -ir only_for_gui="${1}"; shift
 
 	# In copy_filesystem_files, we use `pushd` to changed the working directory into source_fs_mountpoint in order to get proper source and target file path, proactively `popd` to ensure we are not in source_fs_mountpoint and preventing source filesystem to unmount
 	popd &>/dev/null\
 		|| true
 
-	if [ -e "${source_fs_mountpoint}" ]; then
-		echo_with_color green "Unmounting and removing '${source_fs_mountpoint}'..."
-		if umount "${source_fs_mountpoint}"; then
-			if ! rmdir "${source_fs_mountpoint}"; then
-				echo_with_color yellow "${FUNCNAME[0]}: Warning: Unable to remove source mountpoint"
-				clean_result='unclean'
-			fi
-		else
-			echo_with_color yellow "${FUNCNAME[0]}: Warning: Unable to unmount source filesystem."
-			clean_result='unclean'
+	if [ -e "${fs_mountpoint}" ]; then
+		printf_with_color \
+			green \
+			'Unmounting and removing "%s"...\n' \
+			"${fs_mountpoint}"
+		if ! umount "${fs_mountpoint}"; then
+			printf_with_color \
+				yellow \
+				'%s: Warning: Unable to unmount "%s".\n' \
+				"${FUNCNAME[0]}" \
+				"${fs_mountpoint}"
+			return 1
 		fi
-	fi
 
-	if [ -e "${target_fs_mountpoint}" ]; then
-	echo_with_color green "Unmounting and removing '${target_fs_mountpoint}'..."
-		if umount "${target_fs_mountpoint}"; then
-			if ! rmdir "${target_fs_mountpoint}"; then
-				echo_with_color yellow "${FUNCNAME[0]}: Warning: Unable to remove target mountpoint"
-				clean_result='unclean'
-			fi
-		else
-			echo_with_color yellow "${FUNCNAME[0]}: Warning: Unable to unmount target filesystem."
-			clean_result='unsafe'
+		if ! rmdir "${fs_mountpoint}"; then
+			printf_with_color \
+				yellow \
+				'%s: Warning: Unable to remove "%s".\n' \
+				"${FUNCNAME[0]}" \
+				"${fs_mountpoint}"
+			return 2
 		fi
 	fi
 
-	if [ "${clean_result}" == 'unclean' ]; then
-		return 2
-	elif [ "${clean_result}" == 'unsafe' ]; then
-		return 3
-	else
-		return 0
-	fi
-}; declare -fr cleanup_mountpoints
+	return 0
+}; declare -fr cleanup_mountpoint
 
 ## This function continuously moves the GUI creation dialog's progressing bar bit by bit, by running `echo pulse` every 0.05 seconds until the next call chooses to turn off
 ## Refer src/MainPanel.cpp MainPanel::OnInstall(wxCommandEvent& event) method for more info
@@ -1740,31 +1722,64 @@ trap_errexit(){
 }; declare -fr trap_errexit
 
 trap_exit(){
+	# Mountpoints aren't successfully removed
+	local flag_unclean=false
+
+	# Target filesystem failed to unmount
+	local flag_unsafe=false
+
+	# FIXME: Why `trigger_wxGenericProgressDialog_pulse off` here?
+	trigger_wxGenericProgressDialog_pulse\
+		off\
+		"${global_only_for_gui}"
+
 	case "${current_state}" in
 		copying-filesystem|finished)
 			workaround_linux_make_writeback_buffering_not_suck\
 				reset
 		;;
 	esac
 
-	if \
-		util_is_parameter_set_and_not_empty\
-			source_fs_mountpoint\
-		&& util_is_parameter_set_and_not_empty\
-			target_fs_mountpoint; then
-		if cleanup_mountpoints\
-			"${source_fs_mountpoint}"\
-			"${target_fs_mountpoint}"\
+	if util_is_parameter_set_and_not_empty\
+		source_fs_mountpoint; then
+		if ! cleanup_mountpoint \
+			"${source_fs_mountpoint}" \
 			"${global_only_for_gui}"; then
-			:
-		elif [ "${?}" == '2' ]; then
-			echo_with_color yellow 'Some mountpoints are not unmount/cleaned successfully and must be done manually'
-		else # $? == 3, target filesystem not unmounted
-			echo_with_color yellow 'We unable to unmount target filesystem for you, please make sure target filesystem is unmounted before detaching to prevent data corruption'
-			echo_with_color yellow 'Some mountpoints are not unmount/cleaned successfully and must be done manually'
+			flag_unclean=true
+		fi
+	fi
+
+	if util_is_parameter_set_and_not_empty\
+		target_fs_mountpoint; then
+		if ! cleanup_mountpoint \
+			"${target_fs_mountpoint}" \
+			"${global_only_for_gui}"; then
+			local -i return_value="${?}"
+
+			flag_unclean=true
+
+			if [ "${return_value}" = 1 ]; then
+				flag_unsafe=true
+			fi
 		fi
 	fi
 
+	if [ "${flag_unclean}" = true ]; then
+		echo_with_color \
+			yellow \
+			'Some mountpoints are not unmount/cleaned successfully and must be done manually'
+	fi
+
+	if [ "${flag_unsafe}" = true ]; then
+		echo_with_color \
+			yellow \
+			'We unable to unmount target filesystem for you, please make sure target filesystem is unmounted before detaching to prevent data corruption'
+	fi
+
+	unset \
+		flag_unclean \
+		flag_unsafe
+
 	if \
 		util_is_parameter_set_and_not_empty\
 			target_device; then