On Sat, Jan 11, 2014 at 11:08:22AM +0000, Ian Campbell wrote: || I've just come across this old bug report and I'm wondering about the || proposed patch. Is it correct to continue on through the body of the || loop when that abootimg -i call has failed? || || I would expect that either there would be a continue or a -z check. || Perhaps (untested): || if ! abootimg="$(LC_ALL=C abootimg -i "$p" 2>/dev/null)"; then || continue || fi || ? || || Or perhaps: || abootimg="$(LC_ALL=C abootimg -i "$p" 2>/dev/null || true)" || if [ -z "$abootimg" ]; then || continue || fi || Or: || abootimg="$(LC_ALL=C abootimg -i "$p" 2>/dev/null || echo "none")" || if [ "$abootimg" = "none" ]; then || continue || fi || || I'm not familiar with abootimg so I don't know what the expect failure || modes actually look like. The manpage says nothing about abootimg failure modes. However, a little probing shows this: #abootimg -i /dev/mmcblk0p2 Android Boot Image Info: * file name = /dev/mmcblk0p2 [block device] * image size = 8388608 bytes (8.00 MB) page size = 2048 bytes * Boot Name = "Ubuntu Boot Img" * kernel size = 3161540 bytes (3.02 MB) ramdisk size = 2884017 bytes (2.75 MB) * load addresses: kernel: 0x10008000 ramdisk: 0x15000000 tags: 0x10000100 * cmdline = mem=448M@0M tegrapart=recovery:300:a00:800,boot:d00:1000:800,mbr:1d00:200:800 root=UUID=ae1a77f6-839f-4d0d-a196-31b1d421ab7a nosplash * id = 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 #echo $? 0 #abootimg -i /dev/mmcblk0p2 | wc 22 73 608 #abootimg -i /dev/mmcblk0p3 /dev/mmcblk0p3: no Android Magic Value /dev/mmcblk0p3: not a valid Android Boot Image. #echo $? 1 #abootimg -i /dev/mmcblk0p3 | wc /dev/mmcblk0p3: no Android Magic Value /dev/mmcblk0p3: not a valid Android Boot Image. 0 0 0 # Conclusion: with correct argument, prints info on stdout, exit status 0 with wrong argument, prints nothing on stdout, exit status 1 Either result (output or exit status) can be used in this case. The proposed code just looks for a relatively distinctive pattern in the output. Empty output will certainly not match this pattern. I think adding an exit status check only serves to obscure what's actually going on. I'd really love this little trivial issue to be resolved, please. Vincent. -- Vincent Zweije <vincent@zweije.nl> | "If you're flamed in a group you <http://www.xs4all.nl/~zweije/> | don't read, does anybody get burnt?" [Xhost should be taken out and shot] | -- Paul Tomblin on a.s.r.
Attachment:
signature.asc
Description: Digital signature