blobutil/inject: massively improved coding style

top-down function order, with specific functions for
each type of blob. startup logic moved into main(),
also split into smaller functions

"write one program that does one thing well"

blobutil is like that, and has this added philosophy:

"write one function that does one thing well"

during the course of this re-factoring, several bugs
and issues were found, that are pre-existing. these
will be corrected in follow-up revisions

1 files changed, 284 insertions, 179 deletions

diff --git a/resources/scripts/blobs/inject b/resources/scripts/blobs/inject
index 2e4e7a7c..3142355b 100755
--- a/resources/scripts/blobs/inject
+++ b/resources/scripts/blobs/inject
@@ -5,54 +5,107 @@
 # SPDX-FileCopyrightText: 2023 Leah Rowe <info@minifree.org>
 # SPDX-License-Identifier: GPL-3.0-only
 
-Fail(){
-	if [ ! -z ${@+x} ]; then
-		printf "\nERROR: ${@}\n"
+archive=""
+_filetype=""
+rom=""
+board=""
+modifygbe=""
+new_mac=""
+release=""
+releasearchive=""
+
+cbdir="coreboot/default"
+cbcfgsdir="resources/coreboot"
+ifdtool="${cbdir}/util/ifdtool/ifdtool"
+cbfstool="${cbdir}/util/cbfstool/cbfstool"
+nvmutil="util/nvmutil/nvm"
+boarddir=""
+pciromsdir="pciroms"
+
+CONFIG_HAVE_MRC=""
+CONFIG_HAVE_ME_BIN=""
+CONFIG_ME_BIN_PATH=""
+CONFIG_KBC1126_FIRMWARE=""
+CONFIG_KBC1126_FW1=""
+CONFIG_KBC1126_FW1_OFFSET=""
+CONFIG_KBC1126_FW2=""
+CONFIG_KBC1126_FW2_OFFSET=""
+CONFIG_VGA_BIOS_FILE=""
+CONFIG_VGA_BIOS_ID=""
+CONFIG_GBE_BIN_PATH=""
+
+main()
+{
+	if [ $# -lt 1 ]; then
+		fail "No options specified."
+	elif [ "${1}" = "listboards" ]; then
+		listboards
+		exit 0
 	fi
 
-	cat <<- EOF
-	USAGE: ./blobutil inject -r [/path/to/rom] -b [boardname] -m [macaddress]
-	Example: ./blobutil inject -r x230_12mb.rom -b x230_12mb
-
-	Adding a macadress to the gbe is optional.
-	If the [-m] parameter is left blank, the gbe will not be touched.
-
-	Type './blobutil inject listboards' to get a list of valid boards
-	EOF
+	archive="${1}"
+	echo "TEST: $archive"
+
+	while getopts r:b:m: option
+	do
+	    case "${option}"
+		in
+		r)rom=${OPTARG};;
+		b)board=${OPTARG};;
+		m)
+			modifygbe=true
+			new_mac=${OPTARG}
+			;;
+	    esac
+	done
 
-	exit 1
+	check_board
+	build_dependencies
+	inject_blobs
 }
 
-Modify_gbe(){
-	rom=${1}
-	printf "changing mac address in gbe to ${new_mac}\n"
-	_gbe_location=${CONFIG_GBE_BIN_PATH#../../}
-
-	if [ ! -f util/nvmutil/nvm ]; then
-		make -C util/nvmutil || Fail 'failed to build nvmutil'
+check_board()
+{
+	if ! check_release ${archive} ; then
+		if [ ! -f "${rom}" ]; then
+			fail "${rom} is not a valid path"
+		elif [ -z ${rom+x} ]; then
+			fail 'no rom specified'
+		elif [ -z ${board+x} ]; then
+			board=$(detect_board ${rom}) \
+					|| fail 'no board specified'
+		fi
+	else
+		release=true
+		releasearchive="${archive}"
+		board=$(detect_board ${archive}) \
+				|| fail 'Could not detect board type'
 	fi
 
-	_gbe_tmp=$(mktemp -t gbeXXXX.bin)
-	cp ${_gbe_location} ${_gbe_tmp}
-	./util/nvmutil/nvm ${_gbe_tmp} setmac ${new_mac} || Fail 'failed to modify mac address\nmake sure the mac address in the correct format'
-	
-	./coreboot/default/util/ifdtool/ifdtool -i GbE:${_gbe_tmp} ${rom} -O ${rom} || exit 1
-
-	rm ${_gbe_tmp}
+	boarddir="${cbcfgsdir}/${board}"
+	if [ ! -d "${boarddir}" ]; then
+		fail "board ${board} not found"
+	fi
 }
 
-listboards() {
-	for boarddir in resources/coreboot/*; do
-		if [ ! -d "${boarddir}" ]; then continue; fi
-		board="${boarddir##resources/coreboot/}"
-		board="${board%/}"
-		printf '%s\n' "${board##*/}"
-	done
+check_release()
+{
+	if [ ! -f "${archive}" ]; then
+		return 1
+	fi
+
+	if [ "${archive##*.}" = "xz" ]; then
+		printf "%s\n" "Release archive ${archive} detected"
+		return 0
+	else
+		return 1
+	fi
 }
 
 # This function tries to determine the board from the filename of the rom.
 # It will only succeed if the filename is not changed from the build/download
-Detect_board(){
+detect_board()
+{
 	path=${1}
 	filename=$(basename ${path})
 	case ${filename} in
@@ -70,102 +123,69 @@ Detect_board(){
 		return 1
 	esac	
 
-	if [ -d "resources/coreboot/${board}/" ]; then
+	if [ -d "${boarddir}/" ]; then
 		printf '%s\n' "${board}"
+		return 0
 	else
 		return 1
 	fi
 }
 
-Patch(){
-rom="${1}"
-set -- "resources/coreboot/${board}/config/*"
-. ${1} 2>/dev/null
-. "resources/coreboot/${board}/board.cfg"
-
-	if [ "$CONFIG_HAVE_MRC" = "y" ]; then
-		printf 'adding mrc\n'
-		./coreboot/default/util/cbfstool/cbfstool ${rom} add -f mrc/haswell/mrc.bin -n mrc.bin -t mrc -b 0x78fe00 || exit 1
+build_dependencies()
+{
+	if [ ! -d ${cbdir} ]; then
+		printf "downloading coreboot\n"
+		./download coreboot default
 	fi
 
-	if [ "${CONFIG_HAVE_ME_BIN}" = "y" ]; then
-		_me_location=${CONFIG_ME_BIN_PATH#../../}
-		printf 'adding intel management engine\n'
-		./coreboot/default/util/ifdtool/ifdtool -i me:${_me_location} ${rom} -O ${rom} || exit 1
+	if [ ! -f "${ifdtool}" ]; then
+		printf "building ifdtool from coreboot\n"
+		./build module cbutils default \
+				|| fail 'could not build ifdtool'
 	fi
 
-	if [ "${CONFIG_KBC1126_FIRMWARE}" = "y" ]; then
-		_ec1_location="${CONFIG_KBC1126_FW1#../../}"
-		_ec1_offset="${CONFIG_KBC1126_FW1_OFFSET}"
-		_ec2_location="${CONFIG_KBC1126_FW2#../../}"
-		_ec2_offset="${CONFIG_KBC1126_FW2_OFFSET}"
-		printf "adding hp kbc1126 ec firmware\n"
-		if [ "${_ec1_offset}" = "" ] || [ "${_ec1_offset}" = "" ];
-		then
-			printf "EC offsets not declared for board: %s\n" \
-					"${board}"
-			exit 1
-		fi
-		if [ "${_ec1_location}" = "" ] || [ "${_ec2_location}" = "" ];
-		then
-			printf "EC firmware path not declared for board: %s\n" \
-					"${board}"
-		fi
-		if [ ! -f "${_ec1_location}" ] || [ ! -f "${_ec2_location}" ];
-		then
-			printf "EC firmware not downloaded for board: %s\n" \
-					"${board}"
-			exit 1
-		fi
-		./coreboot/default/util/cbfstool/cbfstool "${rom}" add -f ${_ec1_location} -n ecfw1.bin -b ${_ec1_offset} -t raw || exit 1
-		./coreboot/default/util/cbfstool/cbfstool "${rom}" add -f ${_ec2_location} -n ecfw2.bin -b ${_ec2_offset} -t raw || exit 1
+	if [ ! -f "${cbfstool}" ]; then
+		printf "building cbfstool from coreboot\n"
+		./build module cbutils default \
+				|| fail 'could not build cbfstool'
 	fi
 
-	if [ "${CONFIG_VGA_BIOS_FILE}" != "" ] \
-				&& [ "${CONFIG_VGA_BIOS_ID}" != "" ]; then
-		_vga_location="${CONFIG_VGA_BIOS_FILE#../../}"
-		_vga_dir="${_vga_location%/*}"
-		_vga_filename="${_vga_location##*/}"
-		printf "adding pci option rom\n"
-		if [ "${_vga_dir}" != "pciroms" ]; then
-			printf "Invalid PCI ROM directory: %s\n" ${_vga_dir}
-			exit 1
-		fi
-		if [ ! -f "${_vga_location}" ]; then
-			printf "No such file exists: %s\n" ${_vga_location}
-			exit 1
-		fi
-		./coreboot/default/util/cbfstool/cbfstool ${rom} \
-			add -f "${_vga_location}" \
-			-n "pci${CONFIG_VGA_BIOS_ID}.rom" \
-			-t optionrom || exit 1
-	fi
+	./blobutil download ${board} || \
+			fail "Could not download blobs for ${board}"
+}
 
-	if [ "${modifygbe}" = "true" ] && ! [ "${release}" = "true" ]; then
-		Modify_gbe ${rom}
+inject_blobs()
+{
+	if [ "${release}" = "true" ]; then
+		echo 'patching release file'
+		patch_release_roms
+	else
+		patch_rom ${rom}
 	fi
 }
 
-Patch_release(){
+patch_release_roms()
+{
 	_tmpdir=$(mktemp -d "/tmp/${board}_tmpXXXX")
 	tar xf "${releasearchive}" -C "${_tmpdir}" || \
-	Fail 'could not extract release archive'
-	
-	for rom in ${_tmpdir}/bin/*/*.rom ; do
-		echo "patching rom $rom"
-		Patch ${rom} || \
-		Fail "could not patch ${rom}"
+	fail 'could not extract release archive'
+
+	for x in ${_tmpdir}/bin/*/*.rom ; do
+		echo "TEST"
+		echo "patching rom $x"
+		patch_rom ${x} || fail "could not patch ${x}"
+		echo "FOO"
 	done
 
-	( cd ${_tmpdir}/bin/*
+	(
+	cd ${_tmpdir}/bin/*
 	sha1sum --status -c blobhashes || \
-	Fail 'ROMs did not match expected hashes'
+	fail 'ROMs did not match expected hashes'
 	)
-	
 
 	if [ "${modifygbe}" = "true" ]; then
-		for rom in ${_tmpdir}/bin/*/*.rom ; do
-			Modify_gbe ${rom}
+		for x in ${_tmpdir}/bin/*/*.rom ; do
+			modify_gbe ${x}
 		done
 	fi
 
@@ -179,83 +199,168 @@ Patch_release(){
 	rm -r "${_tmpdir}"
 }
 
-Check_release(){
-if ! [ -f ${1} ]; then
-	return 1
-fi
+patch_rom()
+{
+	rom="${1}"
+
+	set -- "${boarddir}/config/"*
+	. ${1} 2>/dev/null
+	. "${boarddir}/board.cfg"
+
+	if [ "$CONFIG_HAVE_MRC" = "y" ]; then
+		inject_blob_intel_mrc "${rom}"
+	fi
+
+	if [ "${CONFIG_HAVE_ME_BIN}" = "y" ]; then
+		inject_blob_intel_me "${rom}"
+	fi
+
+	if [ "${CONFIG_KBC1126_FIRMWARE}" = "y" ]; then
+		inject_blob_hp_kbc1126_ec "${rom}"
+	fi
+
+	if [ "${CONFIG_VGA_BIOS_FILE}" != "" ] \
+				&& [ "${CONFIG_VGA_BIOS_ID}" != "" ]; then
+		inject_blob_dell_e6400_vgarom_nvidia
+	fi
+
+	if [ "${modifygbe}" = "true" ] && ! [ "${release}" = "true" ]; then
+		modify_gbe ${rom}
+	fi
+}
+
+inject_blob_intel_mrc()
+{
+	rom="${1}"
 
-_filetype=$(file -b "${1}")
+	printf 'adding mrc\n'
 
-if [ "${_filetype%%,*}" = "XZ compressed data" ]; then
-	printf "%s\n" "Release archive ${1} detected"
-else
-	return 1
-fi
+	# TODO: use offset defined in coreboot configs (don't hardcode)
+	${cbfstool} ${rom} add -f mrc/haswell/mrc.bin -n mrc.bin -t mrc \
+		-b 0x78fe00 || exit 1
 }
 
-if [ "${1}" = "listboards" ]; then
-	listboards
-	exit 0
-fi
-
-# Implementing parameter parsing now so more options can be added later
-while getopts r:b:m: option
-do 
-    case "${option}"
-        in
-        r)rom=${OPTARG};;
-	b)board=${OPTARG};;
-	m)
-		modifygbe=true
-		new_mac=${OPTARG}
-		;;
-    esac
-done
+inject_blob_intel_me()
+{
+	rom="${1}"
+
+	# TODO: check if CONFIG_ME_BIN_PATH is empty
+	_me_location=${CONFIG_ME_BIN_PATH#../../}
+	printf 'adding intel management engine\n'
+	${ifdtool} -i me:${_me_location} ${rom} -O ${rom} || exit 1
+}
+
+inject_blob_hp_kbc1126_ec()
+{
+	rom="${1}"
+
+	_ec1_location="${CONFIG_KBC1126_FW1#../../}"
+	_ec1_offset="${CONFIG_KBC1126_FW1_OFFSET}"
+	_ec2_location="${CONFIG_KBC1126_FW2#../../}"
+	_ec2_offset="${CONFIG_KBC1126_FW2_OFFSET}"
+
+	printf "adding hp kbc1126 ec firmware\n"
+
+	if [ "${_ec1_offset}" = "" ] || [ "${_ec1_offset}" = "" ]; then
+		printf "EC offsets not declared for board: %s\n" \
+				"${board}"
+		exit 1
+	fi
+	if [ "${_ec1_location}" = "" ] || [ "${_ec2_location}" = "" ]; then
+		printf "EC firmware path not declared for board: %s\n" \
+				"${board}"
+	fi
+	if [ ! -f "${_ec1_location}" ] || [ ! -f "${_ec2_location}" ]; then
+		printf "EC firmware not downloaded for board: %s\n" \
+				"${board}"
+		exit 1
+	fi
+
+	${cbfstool} "${rom}" add -f ${_ec1_location} -n ecfw1.bin \
+			-b ${_ec1_offset} -t raw || exit 1
+	${cbfstool} "${rom}" add -f ${_ec2_location} -n ecfw2.bin \
+			-b ${_ec2_offset} -t raw || exit 1
+}
+
+inject_blob_dell_e6400_vgarom_nvidia()
+{
+	rom="${1}"
+
+	_vga_location="${CONFIG_VGA_BIOS_FILE#../../}"
+	_vga_dir="${_vga_location%/*}"
+	_vga_filename="${_vga_location##*/}"
+
+	printf "adding pci option rom\n"
+
+	if [ "${_vga_dir}" != "${pciromsdir}" ]; then
+		printf "Invalid PCI ROM directory: %s\n" ${_vga_dir}
+		exit 1
+	fi
+	if [ ! -f "${_vga_location}" ]; then
+		printf "No such file exists: %s\n" ${_vga_location}
+		exit 1
+	fi
+
+	${cbfstool} ${rom} add -f "${_vga_location}" \
+		-n "pci${CONFIG_VGA_BIOS_ID}.rom" \
+		-t optionrom || exit 1
+}
+
+modify_gbe()
+{
+	printf "changing mac address in gbe to ${new_mac}\n"
+
+	rom=${1}
+	# TODO: check if CONFIG_GBE_BIN_PATH is empty
+	# TODO: check that the gbe file actually exists
+	_gbe_location=${CONFIG_GBE_BIN_PATH#../../}
+
+	if [ ! -f ${nvmutil} ]; then
+		make -C util/nvmutil || fail 'failed to build nvmutil'
+	fi
 
+	_gbe_tmp=$(mktemp -t gbeXXXX.bin)
+	cp ${_gbe_location} ${_gbe_tmp}
+	${nvmutil} "${_gbe_tmp}" setmac ${new_mac} \
+			|| fail 'failed to modify mac address'
 
+	${ifdtool} -i GbE:${_gbe_tmp} "${rom}" \
+			-O "${rom}" || exit 1
+
+	rm -f ${_gbe_tmp}
+}
 
-if ! Check_release ${1} ; then
-	if [ ! -f "${rom}" ]; then
-		Fail "${rom} is not a valid path"
-	elif [ -z ${rom+x} ]; then
-		Fail 'no rom specified'
-	elif [ -z ${board+x} ]; then
-		board=$(Detect_board ${rom}) || \
-		Fail 'no board specified'
+listboards()
+{
+	for boarddir in ${cbcfgsdir}/*; do
+		if [ ! -d "${boarddir}" ]; then continue; fi
+		board="${boarddir##${cbcfgsdir}/}"
+		board="${board%/}"
+		printf '%s\n' "${board##*/}"
+	done
+}
+
+fail()
+{
+	if [ ! -z ${@+x} ]; then
+		printf "\nERROR: ${@}\n"
 	fi
-else
-	release=true
-	releasearchive="${1}"
-	board=$(Detect_board ${1}) || \
-	Fail 'Could not detect board type'
-fi
-
-
-if [ ! -d "resources/coreboot/${board}/" ]; then
-	Fail "board ${board} not found"
-fi
-
-if [ ! -d coreboot/default ]; then
-	printf "downloading coreboot\n"
-	./download coreboot default
-fi
-
-if [ ! -f "coreboot/default/util/ifdtool/ifdtool" ]; then
-printf "building ifdtool from coreboot\n"
-	./build module cbutils default || Fail 'could not build ifdtool'
-fi
-
-if [ ! -f "coreboot/default/util/cbfstool/cbfstool" ]; then
-printf "building cbfstool from coreboot\n"
-./build module cbutils default || Fail 'could not build cbfstool'
-fi
-
-./blobutil download ${board} || \
-Fail "Could not download blobs for ${board}, check network connection"
-
-if [ "${release}" = "true" ]; then
-	echo 'patching release file'
-	Patch_release
-else
-	Patch ${rom}
-fi
+
+	usage
+	exit 1
+}
+
+usage()
+{
+	cat <<- EOF
+	USAGE: ./blobutil inject -r [rom path] -b [boardname] -m [macaddress]
+	Example: ./blobutil inject -r x230_12mb.rom -b x230_12mb
+
+	Adding a macadress to the gbe is optional.
+	If the [-m] parameter is left blank, the gbe will not be touched.
+
+	Type './blobutil inject listboards' to get a list of valid boards
+	EOF
+}
+
+main $@