* Ian Campbell <i...@debian.org> [2016-10-01 08:29]:
> > Looking at the code, all uses of find_dtb_file() check for the result
> > and produce an error if the file doesn't exist, so maybe we should
> > just move the error messages into find_dtb_file().  Then we could
> > tell
> > the user where we were looking.
> > 
> > Ian, any comments?
> 
> Nope, sounds sensible to me!
> 
> Looks like handle_dtb() doesn't produce an error if the file doesn't
> exist -- but it probably should. Need to take care in the postrm.d case
> (where the file might legitimately not be there) though -- fortunately
> it looks like the result of find_dtb_file isn't actually used in that
> half of the if, so it could probably be moved.

Sorry for the delay (again!).  I finally looked into this again.
I came up with the patch below and I get:

> Using DTB: XXXarmada-370-seagate-personal-cloud.dtb
> Couldn't find DTB XXXarmada-370-seagate-personal-cloud.dtb in 
> /usr/lib/linux-image-4.9.0-rc8-armmp or /etc/flash-kernel/dtbs

So that part looks good.

But then I get:

> Installing  into 
> /boot/dtbs/4.9.0-rc8-armmp/XXXarmada-370-seagate-personal-cloud.dtb
> cp: cannot stat '': No such file or directory

It seems the exit called from find_dtb_file() doesn't exit the whole
program.  I know this is normal because it's called in a subshell, but
flash-kernel itself does a "set -e" so I thought any exit should
trigger the whole script to stop.

Ian, any idea?

Here's the patch so far:

diff --git a/functions b/functions
index e050269..acc1a33 100644
--- a/functions
+++ b/functions
@@ -249,7 +249,7 @@ get_dtb_name() {
                ;;
        esac
        if [ -n "$dtb_name" ] ; then
-               echo "DTB: $dtb_name" >&2
+               echo "Using DTB: $dtb_name" >&2
        fi
 }
 
@@ -561,18 +561,25 @@ android_flash() {
 }
 
 find_dtb_file() {
+       local dtb
        case "$dtb_name" in
        /*)
-               echo "$dtb_name"
+               dtb="$dtb_name"
+               if [ ! -f "$dtb" ]; then
+                       error "Couldn't find $dtb"
+               fi
                ;;
        *)
-               local dtb=$(find /etc/flash-kernel/dtbs -name $dtb_name 
2>/dev/null | head -n 1)
+               dtb=$(find /etc/flash-kernel/dtbs -name $dtb_name 2>/dev/null | 
head -n 1)
                if [ -z "$dtb" ]; then
                        dtb=$(find /usr/lib/linux-image-$kvers -name $dtb_name 
2>/dev/null | head -n 1)
                fi
-               echo $dtb
+               if [ ! -f "$dtb" ]; then
+                       error "Couldn't find DTB $dtb_name in 
/usr/lib/linux-image-$kvers or /etc/flash-kernel/dtbs"
+               fi
                ;;
        esac
+       echo $dtb
 }
 
 handle_dtb() {
@@ -580,9 +587,8 @@ handle_dtb() {
                return
        fi
 
-       local dtb=$(find_dtb_file)
-       local dtb_name=$(basename $dtb_name)
        if [ "x$FK_KERNEL_HOOK_SCRIPT" = "xpostrm.d" ] ; then
+               local dtb_name=$(basename $dtb_name)
                rm -f "/boot/dtbs/$kvers/$dtb_name"
 
                # This was the old name we installed under. We
@@ -606,26 +612,24 @@ handle_dtb() {
                        rmdir --ignore-fail-on-non-empty /boot/dtbs
                fi
        else
-               if [ -e "$dtb" ]; then
-                       echo "Installing $dtb into /boot/dtbs/$kvers/$dtb_name" 
>&2
-                       mkdir -p /boot/dtbs/$kvers/
-                       cp "$dtb" "/boot/dtbs/$kvers/$dtb_name.new"
-                       backup_and_install \
-                               "/boot/dtbs/$kvers/$dtb_name.new" \
-                               "/boot/dtbs/$kvers/$dtb_name"
-
-                       # Historically we installed the dtb as
-                       # dtb-$kvers, keep it around as an alternative
-                       # for now. Useful for platforms which do not
-                       # set ${fdtfile}
-                       ln -nfs "dtbs/$kvers/$dtb_name" "/boot/dtb-$kvers"
-
-                       # This can be used along with the unversioned
-                       # vmlinuz+initrd.gz e.g. as a fallback option
-                       ln -nfs "dtbs/$kvers/$dtb_name" "/boot/dtb"
-               else
-                       error "Couldn't find $dtb"
-               fi
+               local dtb=$(find_dtb_file)
+               local dtb_name=$(basename $dtb_name)
+               echo "Installing $dtb into /boot/dtbs/$kvers/$dtb_name" >&2
+               mkdir -p /boot/dtbs/$kvers/
+               cp "$dtb" "/boot/dtbs/$kvers/$dtb_name.new"
+               backup_and_install \
+                       "/boot/dtbs/$kvers/$dtb_name.new" \
+                       "/boot/dtbs/$kvers/$dtb_name"
+
+               # Historically we installed the dtb as
+               # dtb-$kvers, keep it around as an alternative
+               # for now. Useful for platforms which do not
+               # set ${fdtfile}
+               ln -nfs "dtbs/$kvers/$dtb_name" "/boot/dtb-$kvers"
+
+               # This can be used along with the unversioned
+               # vmlinuz+initrd.gz e.g. as a fallback option
+               ln -nfs "dtbs/$kvers/$dtb_name" "/boot/dtb"
        fi
 }
 
@@ -864,9 +868,6 @@ case "$method" in
                initrd="$ifile"
                if [ "$dtb_append" = "yes" ]; then
                        dtb=$(find_dtb_file)
-                       if [ ! -f "$dtb" ]; then
-                               error "Couldn't find $dtb"
-                       fi
                        append_dtb "$kernel" "$dtb" "$tmpdir/kernel"
                        kernel="$tmpdir/kernel"
                elif [ -n "$machine_id" ]; then
@@ -945,9 +946,6 @@ case "$method" in
                if [ -n "$boot_dtb_path" ]; then
                        boot_dtb_path="$boot_mnt_dir/$boot_dtb_path"
                        boot_dtb=$(find_dtb_file)
-                       if [ ! -f "$boot_dtb" ]; then
-                               error "Couldn't find $boot_dtb"
-                       fi
                        dtb="$tmpdir/dtb"
                        cp "$boot_dtb" "$dtb"
                        backup_and_install "$dtb" "$boot_dtb_path"

-- 
Martin Michlmayr
http://www.cyrius.com/

Reply via email to