I did commit a few other fw_update fixes, so if you see any problems with it, let me know soon, although I probably won't have time to look until this weekend.
Theo rightly complained that the output from fw_update when the network isn't available isn't very good. $ doas fw_update fw_update:fw_update: firmware.openbsd.org: no address associated with name add none; update none Cannot fetch http://firmware.openbsd.org/firmware/snapshots/SHA256.sig We can improve that somewhat in the common case when the network is down with the patch at the end of this email: $ doas ksh ./fw_update.sh fw_update.sh: firmware.openbsd.org: no address associated with name Cannot fetch http://firmware.openbsd.org/firmware/snapshots/SHA256.sig It's not quite as nice when DNS works, and the connection times out: (I could special case this further though) $ doas ksh ./fw_update.sh fw_update.sh| Cannot fetch http://firmware.openbsd.org/firmware/snapshots/SHA256.sig (timed out) And when specifying firmware files that doesn't exist it's still really terrible. $ doas ksh ./fw_update.sh atus-firmware-1.0p1.tgz intel-firmware-20210608v0.tgz fw_update.sh: add atus: Error retrieving http://firmware.openbsd.org/firmware/snapshots/atus-firmware-1.0p1.tgz: 404 Not Found failed (atus-firmware-1.0p1.tgz); update intel: Error retrieving http://firmware.openbsd.org/firmware/snapshots/intel-firmware-20210608v0.tgz: 404 Not Found failed (intel-firmware-20210608v0.tgz) Cannot fetch http://firmware.openbsd.org/firmware/snapshots/atus-firmware-1.0p1.tgz Cannot fetch http://firmware.openbsd.org/firmware/snapshots/intel-firmware-20210608v0.tgz I can tidy that up a bit if I exit early on errors downloading firmware, but that would mean it never attempted installing a firmware that did exist. (that's not part of this patch) $ doas ksh ./fw_update.sh atus-firmware-1.0p1.tgz intel-firmware-20210608v0.tgz fw_update.sh: add atus: Error retrieving http://firmware.openbsd.org/firmware/snapshots/atus-firmware-1.0p1.tgz: 404 Not Found Cannot fetch http://firmware.openbsd.org/firmware/snapshots/atus-firmware-1.0p1.tgz I'm not sure how much better it's possible to make this without trapping all output from external commands and filtering them ourselves. That though could mean output that just disappers if something goes wrong which seems like a bad failure mode. (I think there are some whitespace fixes I need) Comments, OK? Testing with different setups and different levels of verbosity are welcome. I have been testing some, but it's easy to miss things. (what other failure modes am I not considering?) diff --git a/fw_update.sh b/fw_update.sh index a379d74..151b16c 100644 --- a/fw_update.sh +++ b/fw_update.sh @@ -59,7 +59,7 @@ cleanup() { set +o errexit # ignore errors from killing ftp if [ -d "$FD_DIR" ]; then - echo "" >&"$STATUS_FD" + [ -e "$FD_DIR/status" ] && echo "" >&"$STATUS_FD" exec 4>&- [ -s "$FD_DIR/status" ] && cat "$FD_DIR/status" @@ -112,16 +112,18 @@ fetch() { set -o monitor # make sure ftp gets its own process group ( _flags=-vm + _name=${0##/} case "$VERBOSE" in - 0|1) _flags=-VM ;; - 2) _flags=-Vm ;; + 0) _flags=-VM; _name="" ;; + 1) _flags=-VM ;; + 2) _flags=-Vm ;; esac if [ -x /usr/bin/su ]; then exec /usr/bin/su -s /bin/ksh "$_user" -c \ - "/usr/bin/ftp -N '${0##/}' -D 'Get/Verify' $_flags -o- '$_src'" > "$_dst" + "/usr/bin/ftp -N '$_name' -D 'Get/Verify' $_flags -o- '$_src'" > "$_dst" else exec /usr/bin/doas -u "$_user" \ - /usr/bin/ftp -N "${0##/}" -D 'Get/Verify' $_flags -o- "$_src" > "$_dst" + /usr/bin/ftp -N "$_name" -D 'Get/Verify' $_flags -o- "$_src" > "$_dst" fi ) & FTPPID=$! set +o monitor @@ -165,12 +167,12 @@ fetch() { # a blank file indicating failure. check_cfile() { if [ -e "$CFILE" ]; then - [ -s "$CFILE" ] || return 1 + [ -s "$CFILE" ] || return 2 return 0 fi if ! fetch_cfile; then echo -n > "$CFILE" - return 1 + return 2 fi return 0 } @@ -192,7 +194,7 @@ fetch_cfile() { } verify() { - check_cfile || return 1 + check_cfile || return $? # The installer sha256 lacks -C, do it by hand if ! grep -Fqx "SHA256 (${1##*/}) = $( /bin/sha256 -qb "$1" )" "$CFILE" then @@ -207,7 +209,7 @@ verify() { # if VERBOSE is 0, don't show the checksum failure of an existing file. verify_existing() { local _v=$VERBOSE - check_cfile || return 1 + check_cfile || return $? ((_v == 0)) && "$DOWNLOAD" && _v=1 ( VERBOSE=$_v verify "$@" ) @@ -242,7 +244,7 @@ firmware_in_dmesg() { } firmware_filename() { - check_cfile || return 1 + check_cfile || return $? sed -n "s/.*(\($1-firmware-.*\.tgz\)).*/\1/p" "$CFILE" | sed '$!d' } @@ -514,11 +516,14 @@ if ((VERBOSE)); then exec 4>"${FD_DIR}/status" STATUS_FD=4 else + # We use the existence of the status file to indicate + # whether we should print the final newline. + touch "${FD_DIR}/status" exec 4>"${FD_DIR}/warn" WARN_FD=4 fi -status "${0##*/}:" +status "${0##*/}" if "$DELETE"; then [ "$OPT_F" ] && warn "Cannot use -F and -d" && usage @@ -599,7 +604,20 @@ if [ "${devices[*]:-}" ]; then verify_existing=true if [ "$f" = "$d" ]; then - f=$( firmware_filename "$d" ) || continue + f=$( firmware_filename "$d" ) || { + # Fetching the CFILE here is often the + # first attempt to talk to FWURL + # Fetching the CFILE here is often the + # If it does, no point in continuing. + if (($? > 1)); then + # Clear status, we printed an error instead + rm -f "$FD_DIR/status" + exit 1 + fi + + # If it does, no point in continuing. + continue + } if [ ! "$f" ]; then if "$INSTALL" && unregister_firmware "$d"; then unregister="$unregister,$d" @@ -674,10 +692,10 @@ if [ "${devices[*]:-}" ]; then fi if "$INSTALL"; then - status " add " + status ": add " action=Install else - status " download " + status ": download " action=Download fi
