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 <[email protected]> | "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.
signature.asc
Description: Digital signature
