bin/51335: Multiple etcuptdate -d DESTDIR errors/improvements

[ef%math.uni-bonn.de@localhost]

>Number:         51335
>Category:       bin
>Synopsis:       Multiple etcuptdate -d DESTDIR errors/improvements
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Jul 12 16:55:00 +0000 2016
>Originator:     Edgar FuÃ?
>Release:        NetBSD 7
>Organization:
	Mathematisches Institut der Universität Bonn
>Description:
	Having implemented a similar option in a private version of etcupdate before, I spotted some errors in etcupdate's DESTDIR handling:
	-- the check whether to call install_dir incorrectly tests for directory existence without DESTDIR
	-- diff_and_merge_files incorrectly calls install_checksum with the filename (which is taken to be DESTDIR-relative) prefixed by DESTDIR
	-- diff_and_merge_files computes the checksum of the file under test without prefixing it by DESTDIR
	
	Also, the DESTDIR_BRE code is overly complicated. The filename to go into the output is known beforehand, so it can be computed by simple shell substitutions.

	(And the "verbose" messages are inconsistent wrt. prefixing the filename printed by DESTDIR ("missing", "modified" or not (all other instances).)

	(And, as mentioned on tech-userlevel, all of the NEED_xxx logics don't work because diff_and_merge runs in a subshell.)
	
>How-To-Repeat:
	Code inspection
>Fix:
	--- etcupdate.orig	2016-07-12 18:34:35.000000000 +0200
	+++ etcupdate	2016-07-12 18:41:39.000000000 +0200
	@@ -47,7 +47,6 @@
	 # Default settings
	 PROG="${0##*/}"
	 DESTDIR=""		# must not have a trailing slash
	-DESTDIR_BRE=""		# basic regex to match ${DESTDIR}
	 TEMPROOT="${TEMPROOT:=/tmp/temproot}"
	 PAGER="${PAGER:=/usr/bin/more}"
	 SWIDTH="$(stty size | awk '{w=$2}END{if(w==0){w=80}print w}')"
	@@ -177,20 +176,6 @@
		printf "%s\n" "$result"
	 )}
	 
	-# Convert arg $1 to a basic regular expression (as in sed)
	-# that will match the arg.  This works by inserting backslashes
	-# before characters that are special in basic regular expressions.
	-# It also inserts backslashes before the extra characters specified
	-# in $2 (which defaults to "/,").
	-# XXX: Does not handle embedded newlines.
	-# Usage: regex="$(bre_quote "${string}")"
	-bre_quote()
	-{
	-	local arg="$1"
	-	local extra="${2-/,}"
	-	printf "%s\n" "${arg}" | sed -e 's/[][^$.*\\'"${extra}"']/\\&/g'
	-}
	-
	 install_dir() {
		# $1 = target directory (relative to ${DESTDIR})
	 
	@@ -240,10 +225,12 @@
		mkdir -p "${DESTDIR}/var/etcupdate/${D}"
		verbose "Saving MD5 checksum for ${DESTDIR}${1} to" \
		    "${DESTDIR}/var/etcupdate/${1}"
	-	# The sed part of the following pipeline changes things like
	+	# Change things like
		# "MD5 (/path/to/dest/dir/etc/filename) = abc123" to
		# "MD5 (/etc/filename) = abc123".
	-	md5 "${DESTDIR}${1}" | sed -e "s,(${DESTDIR_BRE},(," \
	+	SUM1="$(md5 "${DESTDIR}${1}")"
	+	SUM1="${SUM1##*= }"
	+	printf "MD5 (%s) = %s\n" "${1}" "${SUM1}" \
		    > "${DESTDIR}/var/etcupdate/${1}"
	 }
	 
	@@ -280,12 +267,12 @@
		if cmp -s "${TEMPROOT}${1}" "${DESTDIR}${1}"; then
			verbose "===> ${1} (ok)"
			rm -f "${TEMPROOT}${1}"
	-		install_checksum "${DESTDIR}${1}"
	+		install_checksum "${1}"
			return
		fi
	 
		if ${AUTOMATIC} && [ -f "${DESTDIR}/var/etcupdate/${1}" ]; then
	-		SUM1="$(md5 "${1}")"
	+		SUM1="$(md5 "${DESTDIR}${1}")"
			SUM2="$(cat "${DESTDIR}/var/etcupdate/${1}")"
			if [ "${SUM1}" = "${SUM2}" ]; then
				install_file "${1}"
	@@ -537,7 +524,6 @@
		exit 1
	 fi
	 DESTDIR="${DESTDIR%/}" # remove trailing slash, if any.  result might be "".
	-DESTDIR_BRE="$(bre_quote "${DESTDIR}")"
	 if [ "${N_SRC_ARGS}" -gt 1 ] && ( ${SOURCEMODE} || ${BINARYDIRMODE} ); then
		echo "*** ERROR: Multiple -s args are allowed only with tgz files"
		usage
	@@ -670,7 +656,7 @@
		while read i; do
			D="${i#"${TEMPROOT}"}"
			[ "x${i}" = "x${TEMPROOT}" ] && continue
	-		[ ! -d "${D}" ] && install_dir "${D}" <&3
	+		[ ! -d "${DESTDIR}${D}" ] && install_dir "${D}" <&3
		done
	 fi