Linaro Image Tools

Merge lp:~dave-martin-arm/linaro-image-tools/miscellaneous-fixes into lp:linaro-image-tools/11.11

  1. miscellaneous-fixes
  2. Merge into trunk
Proposed by Dave Martin
Status: Merged
Merged at revision: 136
Proposed branch: lp:~dave-martin-arm/linaro-image-tools/miscellaneous-fixes
Merge into: lp:linaro-image-tools/11.11
Diff against target: 175 lines (+31/-28)
2 files modified
linaro-hwpack-install (+1/-1)
linaro-media-create (+30/-27)
To merge this branch: bzr merge lp:~dave-martin-arm/linaro-image-tools/miscellaneous-fixes
Reviewer Review Type Date Requested Status
James Westby (community) quoting Approve
Linaro Maintainers Pending
Review via email: mp+37950@code.launchpad.net

Description of the change

Minor fixes:
 * make argument quoting a bit more robust so that if the argument to --image_file, --hwpack or --binary contains whitespace or metacharacters, things still work
 * add missing ensure_command dependency for qemu-img.
 * fix an fdisk -l output parsing issue for partitions with odd sector counts
 * fix an off-by-one-sector partition sizing error

To post a comment you must log in.
lp:~dave-martin-arm/linaro-image-tools/miscellaneous-fixes updated
137. By Dave Martin

Fixed misparse of fdisk output.

When a partition has an odd number of sectors, the size field printed
by fdisk is "<size in KiB>+", not "<size in KiB>".

This causes a syntax error when the extracted field is pasted into
an expression to evaluate.

The fix is to subtract the start sector index from end end sector index
(as is already done for the linux partition).

138. By Dave Martin

Fix off-by-one-sector error determining partition sizes from fdisk output.

fdisk lists start sector and end sector indices: the first sector
not in a given partition is <end sector>+1. Thus the correct sector count
for the partition is <end sector>+1-<start sector>.

Revision history for this message
James Westby (james-w) wrote :

106 + sudo cp "$LINARO_HWPACK_INSTALL" ${chroot}/usr/bin

Might as well quote the second argument too.

121 + sudo rm -f ${chroot}/"$(basename "$HWPACK_FILE")"

I would prefer "${chroot}/$(basename "$HWPACK_FILE")"

I haven't reviewed the functional changes.

Thanks,

James

review: Approve (quoting)
Revision history for this message
Matt Waddel (mwaddel) wrote :

Added James' 2 suggestions
Tested on a qemu image deploy and an mmc card deploy with a file that included a space (ie. HW vexpress.tar.gz)
Merged with trunk

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'linaro-hwpack-install'
2--- linaro-hwpack-install 2010-09-20 16:09:22 +0000
3+++ linaro-hwpack-install 2010-10-08 14:40:14 +0000
4@@ -40,7 +40,7 @@
5 elif [ $# -eq 1 ]; then
6 HWPACK_TARBALL=$1
7 elif [ $# -eq 2 ]; then
8- if [ $1 != "--install-latest" ]; then
9+ if [ "$1" != "--install-latest" ]; then
10 die "Unknown argument: $1 \n$usage_msg"
11 fi
12 INSTALL_LATEST="yes"
13
14=== modified file 'linaro-media-create'
15--- linaro-media-create 2010-10-07 15:41:49 +0000
16+++ linaro-media-create 2010-10-08 14:40:14 +0000
17@@ -164,42 +164,42 @@
18
19 # parse commandline options
20 while [ ! -z "$1" ]; do
21- case $1 in
22+ case "$1" in
23 -h|--help)
24 usage
25 ;;
26 --hwpack)
27- checkparm $2
28+ checkparm "$2"
29 HWPACK_FILE="$2"
30 ;;
31 --mmc)
32- checkparm $2
33+ checkparm "$2"
34 MMC="$2"
35 check_mmc
36 ;;
37 --image_file)
38- checkparm $2
39+ checkparm "$2"
40 IMAGE_FILE="$2"
41 ;;
42 --image_size)
43- checkparm $2
44+ checkparm "$2"
45 IMAGE_SIZE=$2
46 ;;
47 --rootfs)
48- checkparm $2
49+ checkparm "$2"
50 RFS="$2"
51 check_fs_type
52 ;;
53 --boot_label)
54- checkparm $2
55+ checkparm "$2"
56 BOOT_LABEL="$2"
57 ;;
58 --rfs_label)
59- checkparm $2
60+ checkparm "$2"
61 RFS_LABEL="$2"
62 ;;
63 --swap_file)
64- checkparm $2
65+ checkparm "$2"
66 SWAP_SIZE="$2"
67 CREATE_SWAP=1
68 ;;
69@@ -211,15 +211,15 @@
70 IS_LOWMEM=1
71 ;;
72 --console)
73- checkparm $2
74+ checkparm "$2"
75 consoles="$consoles $2"
76 ;;
77 --dev)
78- checkparm $2
79+ checkparm "$2"
80 DEVIMAGE=$2
81 ;;
82 --binary)
83- checkparm $2
84+ checkparm "$2"
85 BINARY_TARBALL="$2"
86 ;;
87 esac
88@@ -253,7 +253,7 @@
89 RFS_UUID=`uuidgen -r`
90
91 get_mmcs_by_id() {
92- if [ ! ${IMAGE_FILE} ]; then
93+ if [ ! "${IMAGE_FILE}" ]; then
94 for device in /dev/disk/by-id/*; do
95 if [ `realpath $device` = $MMC ]; then
96 case "$device" in
97@@ -304,15 +304,16 @@
98 ;;
99 *)
100 ensure_command qemu-arm-static qemu-arm-static
101+ ensure_command qemu-img qemu-kvm
102 sudo cp /usr/bin/qemu-arm-static ${chroot}/usr/bin
103 ;;
104 esac
105- sudo cp $LINARO_HWPACK_INSTALL ${chroot}/usr/bin
106+ sudo cp "$LINARO_HWPACK_INSTALL" ${chroot}/usr/bin
107 sudo cp "$HWPACK_FILE" "$chroot"
108
109 # Actually install the hwpack.
110 sudo mount proc ${chroot}/proc -t proc
111- sudo LC_ALL=C chroot "$chroot" linaro-hwpack-install /$(basename "$HWPACK_FILE")
112+ sudo LC_ALL=C chroot "$chroot" linaro-hwpack-install /"$(basename "$HWPACK_FILE")"
113
114 # Revert some changes we did to the rootfs as we don't want them in the
115 # image.
116@@ -323,14 +324,14 @@
117 sudo rm -f ${chroot}/usr/bin/qemu-arm-static
118 fi
119 sudo rm -f ${chroot}/usr/bin/linaro-hwpack-install
120- sudo rm -f ${chroot}/$(basename "$HWPACK_FILE")
121+ sudo rm -f ${chroot}/"$(basename "$HWPACK_FILE")"
122 }
123
124 unpack_binary_tarball() {
125 # Remove the binary/ directory so that previous runs don't interfere here.
126 remove_binary_dir
127
128- sudo tar -xf $BINARY_TARBALL
129+ sudo tar -xf "$BINARY_TARBALL"
130 }
131
132 create_boot_cmd() {
133@@ -418,14 +419,16 @@
134 ) | sudo sfdisk -D -H $HEADS -S $SECTORS $CYLINDER_ARG $partdev
135
136 if [ "${IMAGE_FILE}" ]; then
137- VFATOFFSET=$(($(LC_ALL=C fdisk -l -u $IMAGE_FILE | grep FAT | awk '{print $3}')*512))
138- VFATSIZE=$(($(LC_ALL=C fdisk -l -u $IMAGE_FILE | grep FAT | awk '{print $5}')*1024))
139- ROOTOFFSET=$(($(LC_ALL=C fdisk -l -u $IMAGE_FILE | grep Linux | awk '{print $2}')*512))
140- ROOTSIZE2=$(($(LC_ALL=C fdisk -l -u $IMAGE_FILE | grep Linux | awk '{print $3}')))
141- ROOTSIZE1=$(($(LC_ALL=C fdisk -l -u $IMAGE_FILE | grep Linux | awk '{print $2}')))
142- ROOTSIZE=$((((ROOTSIZE2-ROOTSIZE1)/2)*1024))
143- MMC1=$(sudo losetup -f --show $IMAGE_FILE --offset $VFATOFFSET --sizelimit $VFATSIZE)
144- MMC2=$(sudo losetup -f --show $IMAGE_FILE --offset $ROOTOFFSET --sizelimit $ROOTSIZE)
145+ VFATOFFSET=$(($(LC_ALL=C fdisk -l -u "$IMAGE_FILE" | grep FAT | awk '{print $3}')*512))
146+ VFATSIZE2=$(($(LC_ALL=C fdisk -l -u "$IMAGE_FILE" | grep FAT | awk '{print $4}')))
147+ VFATSIZE1=$(($(LC_ALL=C fdisk -l -u "$IMAGE_FILE" | grep FAT | awk '{print $3}')))
148+ VFATSIZE=$((((VFATSIZE2+1-VFATSIZE1)/2)*1024))
149+ ROOTOFFSET=$(($(LC_ALL=C fdisk -l -u "$IMAGE_FILE" | grep Linux | awk '{print $2}')*512))
150+ ROOTSIZE2=$(($(LC_ALL=C fdisk -l -u "$IMAGE_FILE" | grep Linux | awk '{print $3}')))
151+ ROOTSIZE1=$(($(LC_ALL=C fdisk -l -u "$IMAGE_FILE" | grep Linux | awk '{print $2}')))
152+ ROOTSIZE=$((((ROOTSIZE2+1-ROOTSIZE1)/2)*1024))
153+ MMC1=$(sudo losetup -f --show "$IMAGE_FILE" --offset $VFATOFFSET --sizelimit $VFATSIZE)
154+ MMC2=$(sudo losetup -f --show "$IMAGE_FILE" --offset $ROOTOFFSET --sizelimit $ROOTSIZE)
155 fi
156 }
157
158@@ -591,7 +594,7 @@
159 }
160
161 setup_image() {
162- sudo qemu-img create -f raw $IMAGE_FILE $IMAGE_SIZE
163+ sudo qemu-img create -f raw "$IMAGE_FILE" $IMAGE_SIZE
164 }
165
166 remove_binary_dir() {
167@@ -713,7 +716,7 @@
168
169 if [ "${IMAGE_FILE}" ]; then
170 echo "Create ${IMAGE_FILE}.gz"
171- gzip -f ${IMAGE_FILE} > ${IMAGE_FILE}.gz
172+ gzip -f "${IMAGE_FILE}" > "${IMAGE_FILE}".gz
173 fi
174
175 remove_binary_dir

Subscribers

People subscribed via source and target branches