[pacman-dev] [PATCH 1/4] makepkg: cleanup a few format string injections
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- scripts/makepkg.sh.in | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index d0951df..5a74b3e 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1204,16 +1204,16 @@ check_checksums() { for file in "${source[@]}"; do local found=1 file="$(get_filename "$file")" - printf "%s" " $file ... " >&2 + printf ' %s ... ' "$file" >&2 if [[ ${integrity_sums[$idx]} = 'SKIP' ]]; then - echo "$(gettext "Skipped")" >&2 + printf '%s\n' "$(gettext "Skipped")" >&2 idx=$((idx + 1)) continue fi if ! file="$(get_filepath "$file")"; then - printf -- "$(gettext "NOT FOUND")\n" >&2 + printf '%s\n' "$(gettext "NOT FOUND")" >&2 errors=1 found=0 fi @@ -1223,9 +1223,9 @@ check_checksums() { local realsum="$(openssl dgst -${integ} "$file")" realsum="${realsum##* }" if [[ $expectedsum = "$realsum" ]]; then - printf -- "$(gettext "Passed")\n" >&2 + printf '%s\n' "$(gettext "Passed")" >&2 else - printf -- "$(gettext "FAILED")\n" >&2 + printf '%s\n' "$(gettext "FAILED")" >&2 errors=1 fi fi -- 1.8.4
With some simple math and printf formatting tokens, we can create the whitespace necessary for this without the need for a loop and string concatentation. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- scripts/makepkg.sh.in | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 5a74b3e..2239913 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1149,12 +1149,6 @@ generate_checksums() { local numsrc=${#source[@]} printf "%s" "${integ}sums=(" - local i - local indent='' - for (( i = 0; i < ${#integ} + 6; i++ )); do - indent="$indent " - done - local netfile for netfile in "${source[@]}"; do local proto sum @@ -1176,10 +1170,11 @@ generate_checksums() { ;; esac - (( ct )) && printf "%s" "$indent" - printf "%s" "'$sum'" - ct=$(($ct+1)) - (( $ct < $numsrc )) && echo + # indent checksum on lines after the first + printf "%*s%s" $(( ct ? ${#integ} + 6 : 0 )) '' "'$sum'" + + # print a newline on lines before the last + (( ++ct < numsrc )) && echo done echo ")" -- 1.8.4
These are all cases where we're reading filenames -- any backslashes are intentional and should not be interpreted. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- scripts/makepkg.sh.in | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 2239913..f92f658 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1533,7 +1533,7 @@ strip_file() { objcopy --add-gnu-debuglink="$dbgdir/${binary#/}.debug" "$binary" # create any needed hardlinks - while read -d '' file ; do + while read -rd '' file ; do if [[ "${binary}" -ef "${file}" && ! -f "$dbgdir/${file}.debug" ]]; then mkdir -p "$dbgdir/${file%/*}" ln "$dbgdir/${binary}.debug" "$dbgdir/${file}.debug" @@ -1615,7 +1615,7 @@ tidy_install() { while read -rd ' ' inode; do read file find ${MAN_DIRS[@]} -type l 2>/dev/null | - while read link ; do + while read -r link ; do if [[ "${file}" -ef "${link}" ]] ; then rm -f "$link" "${link}.gz" if [[ ${file%/*} = ${link%/*} ]]; then @@ -1649,7 +1649,7 @@ tidy_install() { fi local binary strip_flags - find . -type f -perm -u+w -print0 2>/dev/null | while read -d '' binary ; do + find . -type f -perm -u+w -print0 2>/dev/null | while read -rd '' binary ; do case "$(file -bi "$binary")" in *application/x-sharedlib*) # Libraries (.so) strip_flags="$STRIP_SHARED";; @@ -1667,7 +1667,7 @@ tidy_install() { if check_option "upx" "y"; then msg2 "$(gettext "Compressing binaries with %s...")" "UPX" local binary - find . -type f -perm -u+w 2>/dev/null | while read binary ; do + find . -type f -perm -u+w 2>/dev/null | while read -r binary ; do if [[ $(file -bi "$binary") = *'application/x-executable'* ]]; then upx $UPXFLAGS "$binary" &>/dev/null || warning "$(gettext "Could not compress binary : %s")" "${binary/$pkgdir\//}" @@ -1695,7 +1695,7 @@ find_libdepends() { local libdeps filename soarch sofile soname soversion; declare -A libdeps; - while read filename; do + while read -r filename; do # get architecture of the file; if soarch is empty it's not an ELF binary soarch=$(LC_ALL=C readelf -h "$filename" 2>/dev/null | sed -n 's/.*Class.*ELF\(32\|64\)/\1/p') [[ -n "$soarch" ]] || continue -- 1.8.4
These loops already maintain an independent loop counter, so cut out the middle man. An extra fix is needed to continue supporting sparse arrays which, while weird, are valid. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- If we don't care about supporting sparse arrays, I can simply drop the final hunk off of this patch and adjust the commit message. Keeping support was simple enough that I felt compelled to offer it. Note, however, that removing support then causes makepkg to fail in cryptic ways when it does encounter a sparse array, e.g.: $ makepkg -g ... ==> Generating checksums for source files... md5sums=('SKIP' 'e99e9189aa2f6084ac28b8ddf605aeb8' 'fb37e34ea006c79be1c54cbb0f803414' ==> ERROR: Unable to find source file . Aborting... And if you're still lost, a sparse array could be constructed like this: source=('git://anongit.freedesktop.org/systemd/systemd.git' 'initcpio-hook-udev' 'initcpio-install-udev') source[5]='initcpio-install-systemd' md5sums=('SKIP' 'e99e9189aa2f6084ac28b8ddf605aeb8' 'fb37e34ea006c79be1c54cbb0f803414') md5sums[5]='fbea6190413f5e54a4d0784ca4a340e4' This is perhaps a contrived example, but it's valid, and today's makepkg handles it just fine. scripts/makepkg.sh.in | 48 +++++++++++++++++++++--------------------------- 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index f92f658..3197e62 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1145,12 +1145,11 @@ generate_checksums() { exit 1 # $E_CONFIG_ERROR fi - local ct=0 - local numsrc=${#source[@]} + local idx numsrc=${#source[@]} printf "%s" "${integ}sums=(" - local netfile - for netfile in "${source[@]}"; do + for (( idx = 0; idx < numsrc; i++ )); do + local netfile=${source[idx]} local proto sum proto="$(get_protocol "$netfile")" @@ -1171,10 +1170,10 @@ generate_checksums() { esac # indent checksum on lines after the first - printf "%*s%s" $(( ct ? ${#integ} + 6 : 0 )) '' "'$sum'" + printf "%*s%s" $(( idx ? ${#integ} + 6 : 0 )) '' "'$sum'" # print a newline on lines before the last - (( ++ct < numsrc )) && echo + (( ++idx < numsrc )) && echo done echo ")" @@ -1193,39 +1192,31 @@ check_checksums() { if (( ${#integrity_sums[@]} == ${#source[@]} )); then msg "$(gettext "Validating source files with %s...")" "${integ}sums" correlation=1 - local errors=0 - local idx=0 - local file - for file in "${source[@]}"; do - local found=1 - file="$(get_filename "$file")" + local idx errors=0 + for (( idx = 0; idx < ${#source[*]}; idx++ )); do + local file="$(get_filename "${source[idx]}")" printf ' %s ... ' "$file" >&2 - if [[ ${integrity_sums[$idx]} = 'SKIP' ]]; then + if [[ ${integrity_sums[idx]} = 'SKIP' ]]; then printf '%s\n' "$(gettext "Skipped")" >&2 - idx=$((idx + 1)) continue fi if ! file="$(get_filepath "$file")"; then printf '%s\n' "$(gettext "NOT FOUND")" >&2 errors=1 - found=0 + continue fi - if (( $found )) ; then - local expectedsum="${integrity_sums[idx],,}" - local realsum="$(openssl dgst -${integ} "$file")" - realsum="${realsum##* }" - if [[ $expectedsum = "$realsum" ]]; then - printf '%s\n' "$(gettext "Passed")" >&2 - else - printf '%s\n' "$(gettext "FAILED")" >&2 - errors=1 - fi + local expectedsum="${integrity_sums[idx],,}" + local realsum="$(openssl dgst -${integ} "$file")" + realsum="${realsum##* }" + if [[ $expectedsum = "$realsum" ]]; then + printf '%s\n' "$(gettext "Passed")" >&2 + else + printf '%s\n' "$(gettext "FAILED")" >&2 + errors=1 fi - - idx=$((idx + 1)) done if (( errors )); then @@ -2798,6 +2789,9 @@ else BUILDFILE="$startdir/$BUILDFILE" fi source_safe "$BUILDFILE" + # rebuild source array to avoid problems with sparse arrays during + # checksum verification. + source=("${source[@]}") fi # set defaults if they weren't specified in buildfile -- 1.8.4
On 04/09/13 13:19, Dave Reisner wrote:
These loops already maintain an independent loop counter, so cut out the middle man. An extra fix is needed to continue supporting sparse arrays which, while weird, are valid.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- If we don't care about supporting sparse arrays, I can simply drop the final hunk off of this patch and adjust the commit message. Keeping support was simple enough that I felt compelled to offer it.
Note, however, that removing support then causes makepkg to fail in cryptic ways when it does encounter a sparse array, e.g.:
$ makepkg -g ... ==> Generating checksums for source files... md5sums=('SKIP' 'e99e9189aa2f6084ac28b8ddf605aeb8' 'fb37e34ea006c79be1c54cbb0f803414' ==> ERROR: Unable to find source file .
Would putting quotes around the outputted source file name clear this up? Note that the output of 'makepkg -g' is incorrect (but working - see below) in the case of sparse source arrays.
Aborting...
And if you're still lost, a sparse array could be constructed like this:
source=('git://anongit.freedesktop.org/systemd/systemd.git' 'initcpio-hook-udev' 'initcpio-install-udev') source[5]='initcpio-install-systemd' md5sums=('SKIP' 'e99e9189aa2f6084ac28b8ddf605aeb8' 'fb37e34ea006c79be1c54cbb0f803414') md5sums[5]='fbea6190413f5e54a4d0784ca4a340e4'
This is perhaps a contrived example, but it's valid, and today's makepkg handles it just fine.
Also, this currently works with the above source array... md5sums=('SKIP' 'e99e9189aa2f6084ac28b8ddf605aeb8' 'fb37e34ea006c79be1c54cbb0f803414' 'fbea6190413f5e54a4d0784ca4a340e4') Which I am guess is not supported after this patch. I am in favour of dropping support for sparse source arrays and just clarifying the output when an empty source entry is present. That could just be added in check_sanity. A
participants (2)
-
Allan McRae
-
Dave Reisner