NetBSD Problem Report #51335

From ef@math.uni-bonn.de  Tue Jul 12 16:50:39 2016
Return-Path: <ef@math.uni-bonn.de>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 4A5C07A13F
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 12 Jul 2016 16:50:39 +0000 (UTC)
Message-Id: <20160712165034.BEE345EDD@jade.math.uni-bonn.de>
Date: Tue, 12 Jul 2016 18:50:34 +0200 (CEST)
From: ef@math.uni-bonn.de
Reply-To: ef@math.uni-bonn.de
To: gnats-bugs@gnats.NetBSD.org
Subject: Multiple etcuptdate -d DESTDIR errors/improvements
X-Send-Pr-Version: 3.95

>Number:         51335
>Category:       bin
>Synopsis:       Multiple etcuptdate -d DESTDIR errors/improvements
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kre
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Jul 12 16:55:00 +0000 2016
>Last-Modified:  Tue Jun 18 23:35:00 +0000 2019
>Originator:     Edgar Fuß
>Release:        NetBSD 7
>Organization:
	Mathematisches Institut der Universität Bonn
>Environment:

>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


>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: bin-bug-people->kre
Responsible-Changed-By: kre@NetBSD.org
Responsible-Changed-When: Tue, 18 Jun 2019 23:35:00 +0000
Responsible-Changed-Why:
I am looking into this PR


>Unformatted:

Home	PR Database Search