Improve portion of the dialog(1) API in dialog.subr responsible for

retrieving stored data (for the --menu, --calendar, --timebox, --checklist,
and --radiolist widgets).

When we (Ron McDowell and I) developed the first version of bsdconfig, it
used temporary files to store responses from dialog(1). That hasn't been
true for some very long time, so the need to always store the return status
of dialog(1) and then call some function to clean-up is long-deprecated. The
function that used to do the clean-up was f_dialog_menutag().

We really don't need f_dialog_menutag() for its originally designed purpose,
as all dialog invocations (even when in a sub-shell) do not use temporary
files anymore.

However, we do need to keep f_dialog_menutag() around because it still fills
the need of being able to abstract the procedure for fetching stored data
provided by functions that display the aforementioned widgets.

In re-designing f_dialog_menutag(), four important changes are made:

1. Rename f_dialog_menutag() to f_dialog_menutag_fetch()
2. Introduce the new first-argument of $var_to_set to reduce number of forks
3. Create a corresponding f_dialog_menutag_store() to abstract the storage
4. Offload the sanitization to a new function, f_dialog_data_sanitize()

NOTE: That last one is important. Not all functions need to store their data
for later fetching, meanwhile every invocation of dialog should be sanitized
(as we learned early-on in the i18n-effort -- underlying libraries will spit
warnings to stderr for bad values of $LANG and since dialog outputs its
responses to stderr, we need to sanitize every response of these warnings).

These changes greatly improve readbaility and also improve performance by
reducing unnecessary forking.

8 files changed, 123 insertions, 85 deletions

diff --git a/usr.sbin/bsdconfig/share/device.subr b/usr.sbin/bsdconfig/share/device.subr
index f973bb8..f724af4 100644
--- a/usr.sbin/bsdconfig/share/device.subr
+++ b/usr.sbin/bsdconfig/share/device.subr
@@ -619,6 +619,7 @@ f_device_menu()
 	case $- in *e*) errexit=1; esac
 	set +e
 
+	local mtag
 	while :; do
 		mtag=$( eval $DIALOG \
 			--title \"\$title\"             \
@@ -648,8 +649,7 @@ f_device_menu()
 
 	if [ $retval -eq 0 ]; then
 		# Clean up the output of [X]dialog(1) and return it
-		setvar DIALOG_MENU_$$ "$mtag"
-		mtag=$( f_dialog_menutag )
+		f_dialog_data_sanitize mtag
 		echo "$mtag" >&2
 	fi
 
diff --git a/usr.sbin/bsdconfig/share/dialog.subr b/usr.sbin/bsdconfig/share/dialog.subr
index f906487..bfbe1db 100644
--- a/usr.sbin/bsdconfig/share/dialog.subr
+++ b/usr.sbin/bsdconfig/share/dialog.subr
@@ -107,6 +107,45 @@ DIALOG_TIMEBOX_HEIGHT=6
 
 ############################################################ GENERIC FUNCTIONS
 
+# f_dialog_data_sanitize $var_to_edit ...
+#
+# When using dialog(1) or Xdialog(1) sometimes unintended warnings or errors
+# are generated from underlying libraries. For example, if $LANG is set to an
+# invalid or unknown locale, the warnings from the Xdialog(1) libraries will
+# clutter the output. This function helps by providing a centralied function
+# that removes spurious warnings from the dialog(1) (or Xdialog(1)) response.
+#
+# Simply pass the name of one or more variables that need to be sanitized.
+# After execution, the variables will hold their newly-sanitized data.
+#
+f_dialog_data_sanitize()
+{
+	if [ "$#" -eq 0 ]; then
+		f_dprintf "%s: called with zero arguments" \
+		          f_dialog_response_sanitize
+		return $FAILURE
+	fi
+
+	local __var_to_edit
+	for __var_to_edit in $*; do
+		# Skip warnings and trim leading/trailing whitespace
+		setvar $__var_to_edit "$( f_getvar $__var_to_edit | awk '
+			BEGIN { data = 0 }
+			{
+				if ( ! data )
+				{
+					if ( $0 ~ /^$/ ) next
+					if ( $0 ~ /^Gdk-WARNING \*\*:/ ) next
+					data = 1
+				}
+				print
+			}
+		' )"
+	done
+}
+
+############################################################ TITLE FUNCTIONS
+
 # f_dialog_title [$new_title]
 #
 # Set the title of future dialog(1) ($DIALOG_TITLE) or backtitle of Xdialog(1)
@@ -1622,29 +1661,41 @@ f_dialog_input()
 
 ############################################################ MENU FUNCTIONS
 
-# f_dialog_menutag
+# f_dialog_menutag_store [-s] $text
+#
+# Store some text from a dialog(1) menu to be retrieved later by
+# f_dialog_menutag_fetch(). If the first argument is `-s', the text is
+# sanitized before being stored.
+#
+f_dialog_menutag_store()
+{
+	local sanitize=
+	[ "$1" = "-s" ] && sanitize=1 && shift 1 # -s
+	local text="$1"
+
+	# Sanitize the menutag before storing it if desired
+	[ "$sanitize" ] && f_dialog_data_sanitize text
+
+	setvar DIALOG_MENU_$$ "$text"
+}
+
+# f_dialog_menutag_fetch [$var_to_set]
 #
 # Obtain the menutag chosen by the user from the most recently displayed
-# dialog(1) menu and clean up any temporary files/variables.
+# dialog(1) menu (previously stored with f_dialog_menutag_store() above). If
+# $var_to_set is NULL or missing, output is printed to stdout (which is less
+# recommended due to performance degradation; in a loop for example).
 #
-f_dialog_menutag()
+f_dialog_menutag_fetch()
 {
-	# Skip warnings
-	eval echo \"\$DIALOG_MENU_$$\" | awk '
-		BEGIN { found = 0 }
-		{
-			if ( found ) # ... just spew
-			{
-				print
-				next
-			}
-			if ( $0 ~ /^$/ ) next
-			if ( $0 ~ /^Gdk-WARNING \*\*:/ ) next
-			found = 1
-			print
-		}
-	'
+	local __var_to_set="$1" __cp
+
+	debug= f_getvar DIALOG_MENU_$$ "${__var_to_set:-__cp}" # get the data
 	setvar DIALOG_MENU_$$ "" # scrub memory in case data was sensitive
+
+	# Return the data on standard-out if desired
+	[ "$__var_to_set" ] || echo "$__cp"
+
 	return $SUCCESS
 }
 
diff --git a/usr.sbin/bsdconfig/share/media/any.subr b/usr.sbin/bsdconfig/share/media/any.subr
index 11fe429..3841300 100644
--- a/usr.sbin/bsdconfig/share/media/any.subr
+++ b/usr.sbin/bsdconfig/share/media/any.subr
@@ -94,10 +94,9 @@ f_media_get_type()
 	                        \"\$hline\"  \
 	                        $menu_list
 
-	local dialog_menu
-
+	local mtag
 	while :; do
-		dialog_menu=$( eval $DIALOG \
+		mtag=$( eval $DIALOG \
 			--title \"\$title\"             \
 			--backtitle \"\$btitle\"        \
 			--hline \"\$hline\"             \
@@ -112,10 +111,7 @@ f_media_get_type()
 			2>&1 >&$DIALOG_TERMINAL_PASSTHRU_FD
 		)
 		local retval=$?
-		setvar DIALOG_MENU_$$ "$dialog_menu"
-
-		local mtag
-		mtag=$( f_dialog_menutag )
+		f_dialog_data_sanitize mtag
 		f_dprintf "retval=%s mtag=[%s]" $retval "$mtag"
 
 		if [ $retval -eq 2 ]; then
diff --git a/usr.sbin/bsdconfig/share/media/ftp.subr b/usr.sbin/bsdconfig/share/media/ftp.subr
index 639f7c3..5164bf2 100644
--- a/usr.sbin/bsdconfig/share/media/ftp.subr
+++ b/usr.sbin/bsdconfig/share/media/ftp.subr
@@ -237,8 +237,8 @@ f_dialog_menu_media_ftp()
 	                        \"\$hline\"  \
 	                        $menu_list
 
-	local dialog_menu retval mtag value
-	dialog_menu=$( eval $DIALOG \
+	local mtag
+	mtag=$( eval $DIALOG \
 		--title \"\$title\"             \
 		--backtitle \"\$btitle\"        \
 		--hline \"\$hline\"             \
@@ -248,16 +248,13 @@ f_dialog_menu_media_ftp()
 		$height $width $rows            \
 		$menu_list                      \
 		2>&1 >&$DIALOG_TERMINAL_PASSTHRU_FD
-	)
-	retval=$?
-	setvar DIALOG_MENU_$$ "$dialog_menu"
-	mtag=$( f_dialog_menutag )
-
-	[ $retval -eq 0 ] || return $FAILURE
+	) || return $FAILURE
+	f_dialog_data_sanitize mtag
 
 	case "$mtag" in
 	URL) setvar $VAR_FTP_PATH "other" ;;
 	*)
+		local value
 		value=$( eval f_dialog_menutag2item \"\$mtag\" $menu_list )
 		setvar $VAR_FTP_PATH "ftp://$value"
 	esac
diff --git a/usr.sbin/bsdconfig/share/media/options.subr b/usr.sbin/bsdconfig/share/media/options.subr
index b891956..b7bf2e8 100644
--- a/usr.sbin/bsdconfig/share/media/options.subr
+++ b/usr.sbin/bsdconfig/share/media/options.subr
@@ -208,9 +208,8 @@ f_media_options_menu()
 		                                  \"\$hline\"  \
 		                                  $menu_list
 
-		local dialog_menu
-
-		dialog_menu=$( eval $DIALOG \
+		local mtag
+		mtag=$( eval $DIALOG \
 			--title \"\$title\"              \
 			--backtitle \"\$btitle\"         \
 			--hline \"\$hline\"              \
@@ -227,11 +226,8 @@ f_media_options_menu()
 			2>&1 >&$DIALOG_TERMINAL_PASSTHRU_FD
 		)
 		local retval=$?
-		setvar DIALOG_MENU_$$ "$dialog_menu"
-		defaultitem="$dialog_menu"
-
-		local mtag
-		mtag=$( f_dialog_menutag )
+		f_dialog_data_sanitize mtag
+		defaultitem="$mtag"
 		f_dprintf "retval=%s mtag=[%s]" $retval "$mtag"
 
 		if [ $retval -eq 2 ]; then
diff --git a/usr.sbin/bsdconfig/share/media/tcpip.subr b/usr.sbin/bsdconfig/share/media/tcpip.subr
index 2d817ba..f1b4ac7 100644
--- a/usr.sbin/bsdconfig/share/media/tcpip.subr
+++ b/usr.sbin/bsdconfig/share/media/tcpip.subr
@@ -1399,8 +1399,7 @@ f_device_dialog_tcp()
 					2>&1 >&$DIALOG_TERMINAL_PASSTHRU_FD
 				)
 				local retval=$?
-				setvar DIALOG_MENU_$$ "$cp"
-				cp=$( f_dialog_menutag )
+				f_dialog_data_sanitize cp
 				f_dprintf "retval=%u mtag=[%s]" $retval "$cp"
 
 				if [ $retval -eq 2 ]; then
diff --git a/usr.sbin/bsdconfig/share/mustberoot.subr b/usr.sbin/bsdconfig/share/mustberoot.subr
index 14e846f..3f23e11 100644
--- a/usr.sbin/bsdconfig/share/mustberoot.subr
+++ b/usr.sbin/bsdconfig/share/mustberoot.subr
@@ -105,8 +105,8 @@ f_become_root_via_sudo()
 		                        \"\$hline\"            \
 		                        $menu_list
 
-		local dialog_menu mtag retval
-		dialog_menu=$( eval $DIALOG \
+		local mtag
+		mtag=$( eval $DIALOG \
 			--title \"\$DIALOG_TITLE\"         \
 			--backtitle \"\$DIALOG_BACKTITLE\" \
 			--hline \"\$hline\"                \
@@ -116,12 +116,8 @@ f_become_root_via_sudo()
 			$height $width $rows               \
 			$menu_list                         \
 			2>&1 >&$DIALOG_TERMINAL_PASSTHRU_FD
-		)
-		retval=$?
-		setvar DIALOG_MENU_$$ "$dialog_menu"
-		mtag=$( f_dialog_menutag )
-
-		[ $retval -eq 0 ] || f_die
+		) || f_die
+		f_dialog_data_sanitize mtag
 
 		case "$mtag" in
 		X) # Cancel/Exit
diff --git a/usr.sbin/bsdconfig/share/packages/packages.subr b/usr.sbin/bsdconfig/share/packages/packages.subr
index 9176ecd..3bc37b5 100755
--- a/usr.sbin/bsdconfig/share/packages/packages.subr
+++ b/usr.sbin/bsdconfig/share/packages/packages.subr
@@ -272,8 +272,8 @@ f_package_calculate_rundeps()
 # Dislay the menu of package categories, complete with package counts for each
 # category, accents, and other miscellany. If $defaultitem is non-NULL and
 # matches one of the existing menu-items, it will be pre-highlighted in the
-# menu dialog (HINT: Use f_dialog_menutag() to populate a local variable that
-# is passed as $defaultitem to highlight the user's last selection by default).
+# menu dialog (HINT: Use f_dialog_menutag_fetch() to populate a local variable
+# that is passed as $defaultitem to highlight the user's last selection).
 #
 f_package_menu_categories()
 {
@@ -297,14 +297,15 @@ f_package_menu_categories()
 		$category_list
 	" # End-Quote
 
-	local height width rows dialog_menu
+	local height width rows
 	eval f_dialog_menu_with_help_size height width rows \
 	                                  \"\$DIALOG_TITLE\"     \
 	                                  \"\$DIALOG_BACKTITLE\" \
 	                                  \"\$prompt\"           \
 	                                  \"\$hline\"            \
 	                                  $menu_list
-	dialog_menu=$( eval $DIALOG \
+	local menu_choice
+	menu_choice=$( eval $DIALOG \
 			--title \"\$DIALOG_TITLE\"         \
 			--backtitle \"\$DIALOG_BACKTITLE\" \
 			--hline \"\$hline\"                \
@@ -318,7 +319,7 @@ f_package_menu_categories()
 			2>&1 >&$DIALOG_TERMINAL_PASSTHRU_FD
 		)
 	local retval=$?
-	setvar DIALOG_MENU_$$ "$dialog_menu"
+	f_dialog_menutag_store -s "$menu_choice"
 	return $retval
 }
 
@@ -360,7 +361,7 @@ f_package_index_get_page()
 #
 # On success, if the user doesn't press ESC or choose Cancel, the environment
 # variable $DIALOG_MENUITEM_$$ will hold the item associated with the chosen
-# tag (accessible through f_dialog_menutag()).
+# tag (accessible through f_dialog_menutag_fetch()).
 #
 f_package_menu_select()
 {
@@ -464,8 +465,9 @@ f_package_menu_select()
 	f_dialog_infobox_size iheight iwidth \
 	         	"$DIALOG_TITLE" "$DIALOG_BACKTITLE" \
 	         	"$msg_processing_selection"
-	local dialog_menu item
-	dialog_menu=$( eval $DIALOG \
+
+	local menu_choice
+	menu_choice=$( eval $DIALOG \
 		--title \"\$DIALOG_TITLE\"         \
 		--backtitle \"\$DIALOG_BACKTITLE\" \
 		--hline \"\$hline\"                \
@@ -484,11 +486,13 @@ f_package_menu_select()
 		2>&1 >&$DIALOG_TERMINAL_PASSTHRU_FD
 	)
 	local retval=$?
-	setvar DIALOG_MENU_$$ "$dialog_menu"
+	f_dialog_data_sanitize menu_choice
+	f_dialog_menutag_store "$menu_choice"
 
 	if [ $retval -eq $SUCCESS ]; then
+		local item
 		item=$( eval f_dialog_menutag2item${SHOW_DESC:+_with_help} \
-		             	\"\$dialog_menu\" $menu_list )
+		             	\"\$menu_choice\" $menu_list )
 		setvar DIALOG_MENUITEM_$$ "$item"
 	fi
 
@@ -501,11 +505,11 @@ f_package_menu_select()
 # with regard to "deselecting" an already installed package. Choices include
 # uninstall, re-install, or cancel (leave $package marked as installed).
 # Returns success if the user does not press ESC or choose Cnacel. Use the
-# f_dialog_menutag() function upon success to retrieve the user's choice.
+# f_dialog_menutag_fetch() function upon success to retrieve the user's choice.
 #
 f_package_menu_deselect()
 {
-	local package="$1" prompt menu_list dialog_menu
+	local package="$1" prompt menu_list
 	prompt=$( printf "$msg_what_would_you_like_to_do_with" "$package" )
 	local hline="$hline_alnum_arrows_punc_tab_enter"
 	menu_list="
@@ -521,8 +525,8 @@ f_package_menu_deselect()
 	                        \"\$prompt\"           \
 	                        \"\$hline\"            \
 	                        $menu_list
-
-	dialog_menu=$( eval $DIALOG \
+	local menu_choice
+	menu_choice=$( eval $DIALOG \
 		--title \"\$DIALOG_TITLE\"         \
 		--backtitle \"\$DIALOG_BACKTITLE\" \
 		--hline \"\$hline\"                \
@@ -534,7 +538,7 @@ f_package_menu_deselect()
 		2>&1 >&$DIALOG_TERMINAL_PASSTHRU_FD
 	)
 	local retval=$?
-	setvar DIALOG_MENU_$$ "$dialog_menu"
+	f_dialog_menutag_store -s "$menu_choice"
 	return $retval
 }
 
@@ -547,7 +551,7 @@ f_package_menu_deselect()
 #
 f_package_review()
 {
-	local prompt dialog_menu package varpkg mark menu_list=
+	local prompt package varpkg mark menu_list=
 	prompt=$( printf "$msg_reviewing_selected_packages" \
 	                 "$_All_nselected" )
 	local hline="$hline_alnum_arrows_punc_tab_enter"
@@ -576,7 +580,8 @@ f_package_review()
 	                        \"\$hline\"            \
 	                        $menu_list
 
-	dialog_menu=$( eval $DIALOG \
+	# Show the review menu (ignore menu choice)
+	eval $DIALOG \
 		--title \"\$DIALOG_TITLE\"         \
 		--backtitle \"\$DIALOG_BACKTITLE\" \
 		--hline \"\$hline\"                \
@@ -585,10 +590,8 @@ f_package_review()
 		--menu \"\$prompt\"                \
 		$height $width $rows               \
 		$menu_list                         \
-		2>&1 >&$DIALOG_TERMINAL_PASSTHRU_FD
-	)
+		2> /dev/null
 	local retval=$?
-	setvar DIALOG_MENU_$$ "$dialog_menu"
 
 	#
 	# XXX
@@ -626,7 +629,7 @@ f_package_config()
 		f_package_menu_categories \
 			CATEGORY_MENU_LIST "$category_defaultitem"
 		retval=$?
-		category=$( f_dialog_menutag )
+		f_dialog_menutag_fetch category
 		f_dprintf "retval=%u mtag=[%s]" $retval "$category"
 		category_defaultitem="$category"
 
@@ -648,7 +651,7 @@ f_package_config()
 
 		f_str2varname "$category" varcat
 
-		local page package varpkg mark
+		local page package varpkg mark menu_choice
 		while :; do
 			# Display the list of packages for selected category
 			page=1 defaultitem=""
@@ -657,8 +660,8 @@ f_package_config()
 			f_package_menu_select \
 				"$category" "${page:=1}" "$defaultitem"
 			retval=$?
-			dialog_menu=$( f_dialog_menutag )
-			f_dprintf "retval=%u mtag=[%s]" $retval "$dialog_menu"
+			f_dialog_menutag_fetch menu_choice
+			f_dprintf "retval=%u mtag=[%s]" $retval "$menu_choice"
 
 			# NOTE: When --and-widget is used only ESC will cause
 			# dialog(1) to return without going to the next widget.
@@ -666,24 +669,24 @@ f_package_config()
 			# the Cancel button because stdout will be NULL.
 			# Alternatively, Xdialog(1) will terminate with 1
 			# if/when Cancel is chosen on any widget.
-			if [ $retval -eq 255 -o ! "$dialog_menu" ]; then
+			if [ $retval -eq 255 -o ! "$menu_choice" ]; then
 				# User pressed ESC or chose Cancel
 				break
 			elif [ $retval -eq 1 ]; then
 				# Using X11, Xdialog(1) returned 1 for Cancel
-				f_show_msg "%s" "$dialog_menu"
+				f_show_msg "%s" "$menu_choice"
 				break
 			elif [ $retval -ne $SUCCESS ]; then
 				# X11-related error occurred using Xdialog(1)
-				f_show_msg "%s" "$dialog_menu"
+				f_show_msg "%s" "$menu_choice"
 				break
 			fi
 
-			defaultitem="$dialog_menu"
+			defaultitem="$menu_choice"
 
 			# NOTE: f_package_menu_select() does not show the
 			# `Previous Page' or `Next Page' items unless needed
-			case "$dialog_menu" in
+			case "$menu_choice" in
 			"> $msg_previous_page"|"> $msg_previous_page*")
 				page=$(( $page - 1 ))
 				setvar _defaultpage_$varcat $page
@@ -722,7 +725,7 @@ f_package_config()
 			esac
 
 			# Treat any other selection as a package
-			package="${dialog_menu# }" # Trim leading space
+			package="${menu_choice# }" # Trim leading space
 			f_str2varname $package varpkg
 			f_getvar DIALOG_MENUITEM_$$ mark
 			mark="${mark#?}"
@@ -738,8 +741,8 @@ f_package_config()
 				;;
 			"X"|"R"|"U")
 				f_package_menu_deselect $package || continue
-				dialog_menu=$( f_dialog_menutag )
-				case "$dialog_menu" in
+				f_dialog_menutag_fetch menu_choice
+				case "$menu_choice" in
 				"X $msg_installed")
 					f_package_deselect "$package"
 					mark="X"