commit 0f7b946342e3d40048689d849369e789d052d66a Author: Kerin Millar Date: Wed Feb 7 02:17:32 2018 +0000 flag-o-matic: refactor strip-flags and fix non-functioning -O2 fallback The strip-flags() function tries to add -O2 to the stripped flag list, if it finds that it is reponsible for having stripped all -O flags. This code is defective because it relies on passing the literal name of the array containing the stripped flags to _is_flagq() for inspection, by way of indirection. This cannot possibly work because the array, entitled "new" is locally scoped to the calling function. As such, it is not visible to the _is_flagq() function. From inspecting the _is_flaqq() code, I could see that it was intended for one use case - dereferencing the scalar variables produced by all-flag-vars(). However, it appears to have had support for dereferencing array variables tacked on (in a very unsafe way). The sole case where array dereferencing is relied upon is in passing "new" from strip-flags() and - even then - it doesn't work, as explained above. Therefore, I elected to fix it by instead keeping a pseudo-boolean named has_o. For each flag that is added to the new list, if -O* is encountered, it sets has_o=1. Thus, _is_flagq() is no longer needed to assess whether -O* is missing in the new list. Also, I re-factored the code. It eliminates all potentially unsafe expansions, and no longer requires pathname expansion to be disabled. The read builtin is used to split scalar variables containing lists of flags, and is thus completely immune to the effects of globbing and pathname expansion. It uses an associative array to avoid repeatedly walking flag sequences. Micro-benchmarks indicate that the function is approximately 2.5 times faster. I have also attempted to make the code easier to read by declaring variables in accordance with their type, and by changing some of the variable names. diff --git a/eclass/flag-o-matic.eclass b/eclass/flag-o-matic.eclass index 14b84fbdbebe..e01eb80e2b67 100644 --- a/eclass/flag-o-matic.eclass +++ b/eclass/flag-o-matic.eclass @@ -382,40 +382,37 @@ filter-mfpmath() { # Strip *FLAGS of everything except known good/safe flags. This runs over all # flags returned by all_flag_vars(). strip-flags() { - local x y var - - local ALLOWED_FLAGS + local var flag has_o + local -a ALLOWED_FLAGS old_flags new_flags setup-allowed-flags - set -f # disable pathname expansion + local -A safe + for flag in "${ALLOWED_FLAGS[@]}"; do + safe[$flag]= + done - for var in $(all-flag-vars) ; do - local new=() - - for x in ${!var} ; do - local flag=${x%%=*} - for y in "${ALLOWED_FLAGS[@]}" ; do - if [[ -z ${flag%%${y}} ]] ; then - new+=( "${x}" ) - break + for var in $(all-flag-vars); do + new_flags=() + read -ra old_flags <<<"${!var}" + for flag in "${old_flags[@]}"; do + if [[ -v "safe[${flag%%=*}]" ]]; then + new_flags+=( "$flag" ) + if [[ $flag == -O* ]]; then + has_o=1 fi - done + fi done # In case we filtered out all optimization flags fallback to -O2 - if _is_flagq ${var} "-O*" && ! _is_flagq new "-O*" ; then - new+=( -O2 ) + if (( ! has_o )) && _is_flagq "$var" '-O*'; then + new_flags+=( -O2 ) fi - if [[ ${!var} != "${new[*]}" ]] ; then - einfo "strip-flags: ${var}: changed '${!var}' to '${new[*]}'" + if [[ ${old_flags[*]} != "${new_flags[*]}" ]]; then + einfo "strip-flags: ${var}: changed '${old_flags[*]}' to '${new_flags[*]}'" + export "$var=${new_flags[*]}" fi - export ${var}="${new[*]}" done - - set +f # re-enable pathname expansion - - return 0 } test-flag-PROG() {