diff mbox

[OpenWrt-Devel] procd: reorder the arguments of command invocations

Message ID 1420912491-21359-1-git-send-email-gianluca@sottospazio.it
State Superseded
Headers show

Commit Message

gianluca Jan. 10, 2015, 5:54 p.m. UTC 
Some commands in the script /lib/upgrade/nand.sh (provided by
procd) use option arguments after non-option arguments.

This unfortunately doesn't work with musl because it doesn't yet support
permutations of the arguments: when getopt finds a non-option argument
it returns -1 and ignores any option which may follow.

One visible side effect is that sysupgrade doesn't work with musl/NAND
tarballs.

The following patch reorders the command invocations by placing the
options before any non-option argument.

Tested on a Netgear WNDR4300 built with ubifs support and musl libc.

Signed-off-by: Gianluca Anzolin <gianluca@sottospazio.it>
---
 package/system/procd/files/nand.sh | 40 +++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

Comments

Felix Fietkau Jan. 11, 2015, 12:13 p.m. UTC | #1 
On 2015-01-10 18:54, Gianluca Anzolin wrote:
> Some commands in the script /lib/upgrade/nand.sh (provided by
> procd) use option arguments after non-option arguments.
> 
> This unfortunately doesn't work with musl because it doesn't yet support
> permutations of the arguments: when getopt finds a non-option argument
> it returns -1 and ignores any option which may follow.
> 
> One visible side effect is that sysupgrade doesn't work with musl/NAND
> tarballs.
> 
> The following patch reorders the command invocations by placing the
> options before any non-option argument.
> 
> Tested on a Netgear WNDR4300 built with ubifs support and musl libc.
> 
> Signed-off-by: Gianluca Anzolin <gianluca@sottospazio.it>
It seems that musl git now supports argv permutation. I'm going to merge
the git changes since the last release soon.

- Felix
gianluca Jan. 11, 2015, 12:58 p.m. UTC | #2 
Il 11/01/2015 13:13, Felix Fietkau ha scritto:
> On 2015-01-10 18:54, Gianluca Anzolin wrote:
>> Some commands in the script /lib/upgrade/nand.sh (provided by
>> procd) use option arguments after non-option arguments.
>>
>> This unfortunately doesn't work with musl because it doesn't yet support
>> permutations of the arguments: when getopt finds a non-option argument
>> it returns -1 and ignores any option which may follow.
>>
>> One visible side effect is that sysupgrade doesn't work with musl/NAND
>> tarballs.
>>
>> The following patch reorders the command invocations by placing the
>> options before any non-option argument.
>>
>> Tested on a Netgear WNDR4300 built with ubifs support and musl libc.
>>
>> Signed-off-by: Gianluca Anzolin <gianluca@sottospazio.it>
> It seems that musl git now supports argv permutation. I'm going to merge
> the git changes since the last release soon.
>
> - Felix
Fair enough, thank you.

Gianluca
gianluca Jan. 11, 2015, 1:09 p.m. UTC | #3 
Il 11/01/2015 13:58, Gianluca Anzolin ha scritto:
> Il 11/01/2015 13:13, Felix Fietkau ha scritto:
>> It seems that musl git now supports argv permutation. I'm going to merge
>> the git changes since the last release soon.
>>
>> - Felix
> Fair enough, thank you.
>
> Gianluca

However I'm a bit worried that permutations are supported only for 
getopt_long() not getopt().

 From a cursory look at the source of busybox's tar and ubi-tools it 
seems they use getopt_long() fortunately. But there are packages in 
ubi-tools which use std getopt().

Gianluca
Felix Fietkau Jan. 11, 2015, 6:28 p.m. UTC | #4 
On 2015-01-11 14:09, Gianluca Anzolin wrote:
> Il 11/01/2015 13:58, Gianluca Anzolin ha scritto:
>> Il 11/01/2015 13:13, Felix Fietkau ha scritto:
>>> It seems that musl git now supports argv permutation. I'm going to merge
>>> the git changes since the last release soon.
>>>
>>> - Felix
>> Fair enough, thank you.
>>
>> Gianluca
> 
> However I'm a bit worried that permutations are supported only for 
> getopt_long() not getopt().
> 
>  From a cursory look at the source of busybox's tar and ubi-tools it 
> seems they use getopt_long() fortunately. But there are packages in 
> ubi-tools which use std getopt().
Please test r43939 and check if the scripts work now.

Thanks,

- Felix
gianluca Jan. 11, 2015, 8:05 p.m. UTC | #5 
Il 11/01/2015 19:28, Felix Fietkau ha scritto:
> On 2015-01-11 14:09, Gianluca Anzolin wrote:
>> Il 11/01/2015 13:58, Gianluca Anzolin ha scritto:
>>> Il 11/01/2015 13:13, Felix Fietkau ha scritto:
>>>> It seems that musl git now supports argv permutation. I'm going to merge
>>>> the git changes since the last release soon.
>>>>
>>>> - Felix
>>> Fair enough, thank you.
>>>
>>> Gianluca
>> However I'm a bit worried that permutations are supported only for
>> getopt_long() not getopt().
>>
>>   From a cursory look at the source of busybox's tar and ubi-tools it
>> seems they use getopt_long() fortunately. But there are packages in
>> ubi-tools which use std getopt().
> Please test r43939 and check if the scripts work now.
>
> Thanks,
>
> - Felix
After an endless recompilation I can confirm that in r43939 sysupgrade 
works even without reording the arguments in /lib/upgrade/nand.sh

Thank you.

Gianluca
diff mbox

Patch

diff --git a/package/system/procd/files/nand.sh b/package/system/procd/files/nand.sh
index 7fb9343..a22381d 100644
--- a/package/system/procd/files/nand.sh
+++ b/package/system/procd/files/nand.sh
@@ -58,7 +58,7 @@  nand_get_magic_long() {
 }
 
 get_magic_long_tar() {
-	( tar xf $1 $2 -O | dd bs=4 count=1 | hexdump -v -n 4 -e '1/1 "%02x"') 2> /dev/null
+	( tar -O -xf $1 $2 | dd bs=4 count=1 | hexdump -v -n 4 -e '1/1 "%02x"') 2> /dev/null
 }
 
 identify_magic() {
@@ -132,13 +132,13 @@  nand_upgrade_prepare_ubi() {
 	fi
 
 	if [ ! "$ubidev" ]; then
-		ubiformat /dev/mtd$mtdnum -y
+		ubiformat -y /dev/mtd$mtdnum
 		ubiattach -m "$mtdnum"
 		sync
 		ubidev="$( nand_find_ubi "$CI_UBIPART" )"
 		[ "$has_env" -gt 0 ] && {
-			ubimkvol /dev/$ubidev -n 0 -N ubootenv -s 1MiB
-			ubimkvol /dev/$ubidev -n 1 -N ubootenv2 -s 1MiB
+			ubimkvol -n 0 -N ubootenv -s 1MiB /dev/$ubidev
+			ubimkvol -n 1 -N ubootenv2 -s 1MiB /dev/$ubidev
 		}
 	fi
 
@@ -157,13 +157,13 @@  nand_upgrade_prepare_ubi() {
 	fi
 
 	# kill volumes
-	[ "$kern_ubivol" ] && ubirmvol /dev/$ubidev -N kernel || true
-	[ "$root_ubivol" ] && ubirmvol /dev/$ubidev -N rootfs || true
-	[ "$data_ubivol" ] && ubirmvol /dev/$ubidev -N rootfs_data || true
+	[ "$kern_ubivol" ] && ubirmvol -N kernel /dev/$ubidev || true
+	[ "$root_ubivol" ] && ubirmvol -N rootfs /dev/$ubidev || true
+	[ "$data_ubivol" ] && ubirmvol -N rootfs_data /dev/$ubidev || true
 
 	# update kernel
 	if [ "$has_kernel" = "1" ]; then
-		if ! ubimkvol /dev/$ubidev -N kernel -s $kernel_length; then
+		if ! ubimkvol -N kernel -s $kernel_length /dev/$ubidev ; then
 			echo "cannot create kernel volume"
 			return 1;
 		fi
@@ -176,14 +176,14 @@  nand_upgrade_prepare_ubi() {
 	else
 		root_size_param="-s $rootfs_length"
 	fi
-	if ! ubimkvol /dev/$ubidev -N rootfs $root_size_param; then
+	if ! ubimkvol -N rootfs $root_size_param /dev/$ubidev ; then
 		echo "cannot create rootfs volume"
 		return 1;
 	fi
 
 	# create rootfs_data for non-ubifs rootfs
 	if [ "$rootfs_type" != "ubifs" ]; then
-		if ! ubimkvol /dev/$ubidev -N rootfs_data -m; then
+		if ! ubimkvol -N rootfs_data -m /dev/$ubidev ; then
 			echo "cannot initialize rootfs_data volume"
 			return 1
 		fi
@@ -219,7 +219,7 @@  nand_upgrade_ubinized() {
 	local mtddev="/dev/mtd${mtdnum}"
 	ubidetach -p "${mtddev}" || true
 	sync
-	ubiformat "${mtddev}" -y -f "${ubi_file}"
+	ubiformat -y -f "${mtddev}" "${ubi_file}"
 	ubiattach -p "${mtddev}"
 	nand_do_upgrade_success
 }
@@ -232,7 +232,7 @@  nand_upgrade_ubifs() {
 	
 	local ubidev="$( nand_find_ubi "$CI_UBIPART" )"
 	local root_ubivol="$(nand_find_volume $ubidev rootfs)"
-	ubiupdatevol /dev/$root_ubivol -s $rootfs_length $1
+	ubiupdatevol -s $rootfs_length /dev/$root_ubivol $1
 
 	nand_do_upgrade_success
 }
@@ -242,8 +242,8 @@  nand_upgrade_tar() {
 	local board_name="$(cat /tmp/sysinfo/board_name)"
 	local kernel_mtd="$(find_mtd_index $CI_KERNPART)"
 
-	local kernel_length=`(tar xf $tar_file sysupgrade-$board_name/kernel -O | wc -c) 2> /dev/null`
-	local rootfs_length=`(tar xf $tar_file sysupgrade-$board_name/root -O | wc -c) 2> /dev/null`
+	local kernel_length=`(tar -O -xf $tar_file sysupgrade-$board_name/kernel | wc -c) 2> /dev/null`
+	local rootfs_length=`(tar -O -xf $tar_file sysupgrade-$board_name/root | wc -c) 2> /dev/null`
 
 	local rootfs_type="$(identify_tar "$tar_file" sysupgrade-$board_name/root)"
 
@@ -251,7 +251,7 @@  nand_upgrade_tar() {
 	local has_env=0
 
 	[ "$kernel_length" != 0 -a -n "$kernel_mtd" ] && {
-		tar xf $tar_file sysupgrade-$board_name/kernel -O | mtd write - $CI_KERNPART
+		tar -O -xf $tar_file sysupgrade-$board_name/kernel | mtd write - $CI_KERNPART
 	}
 	[ "$kernel_length" = 0 -o ! -z "$kernel_mtd" ] && has_kernel=0
 
@@ -260,13 +260,13 @@  nand_upgrade_tar() {
 	local ubidev="$( nand_find_ubi "$CI_UBIPART" )"
 	[ "$has_kernel" = "1" ] && {
 		local kern_ubivol="$(nand_find_volume $ubidev kernel)"
-	 	tar xf $tar_file sysupgrade-$board_name/kernel -O | \
-			ubiupdatevol /dev/$kern_ubivol -s $kernel_length -
+		tar -O -xf $tar_file sysupgrade-$board_name/kernel | \
+			ubiupdatevol -s $kernel_length /dev/$kern_ubivol -
 	}
 
 	local root_ubivol="$(nand_find_volume $ubidev rootfs)"
-	tar xf $tar_file sysupgrade-$board_name/root -O | \
-		ubiupdatevol /dev/$root_ubivol -s $rootfs_length -
+	tar -O -xf $tar_file sysupgrade-$board_name/root | \
+		ubiupdatevol -s $rootfs_length /dev/$root_ubivol -
 
 	nand_do_upgrade_success
 }
@@ -340,7 +340,7 @@  append sysupgrade_pre_upgrade nand_upgrade_stage1
 nand_do_platform_check() {
 	local board_name="$1"
 	local tar_file="$2"
-	local control_length=`(tar xf $tar_file sysupgrade-$board_name/CONTROL -O | wc -c) 2> /dev/null`
+	local control_length=`(tar -O -xf $tar_file sysupgrade-$board_name/CONTROL | wc -c) 2> /dev/null`
 	local file_type="$(identify $2)"
 
 	[ "$control_length" = 0 -a "$file_type" != "ubi" -a "$file_type" != "ubifs" ] && {