2

I want to know if the code below can be improved of how it is written is the way to write it. If it wasn't clear I am a newbie to bash but my script completes its purpose.

This code is trying to define the variables s$counter= s1, s2, s3, s4... in terms of the previously defined variables s1$j and s2$j with j a string that is an item of a list (all defined previously).

counter=1 
for j in ${list[@]}
    do
            eval s$counter=$(eval "echo \$s1$j")
            eval s$((counter+1))=$(eval "echo \$s2$j")
            counter=$((counter+2))
    done

The inner eval, meaning $(eval "echo \$s1$j") es meant to return the value of s1$j. The second eval, meaning eval s$counter =... is meant to define the variables s1, s2, s3, s4 ...

So an example would be: let's say for the first foor loop j=a then $(eval "echo \$s1$j") == $s1a, with the value of s1a defined earlier in the script, for example let it be "s1a=10", so that when the second eval is evaluated we have the command that states "s1=10".

It works, but could something happen that would make this a possible memory threat or something like that?.

The same idea with this line of code.

eval $(echo "sed -i '$(eval echo '17i$sed17line')' $file")

where sed17line is what I want to add in the line 17 of file. It Depends on what I want to use the script for, which is why I am using it as a variable and hence why I am using a combination of echo and eval.

2
  • Bash has an array construct tldp.org/LDP/Bash-Beginners-Guide/html/sect_10_02.html Normally simple list processing should be all you need however. Also, one eval per line is enough. Eval is just treating the expanded string as if it were in the script. – T Nierath May 16 '18 at 19:20
  • You are right! Only one eval is necessary. Thanks! – M.O. May 16 '18 at 19:42
1

It's good be aware there are issues with eval (this and this) and echo (here). You should avoid eval, it's easy to misuse.

Even if these commands are completely safe in your particular cases here, I advise you to drop eval. Your code is indeed over-complicated because of it.


Your first example gets much cleaner with arrays. This works in GNU bash 4.4.12 in my Debian:

# preparing variables
unset -v list s s1 s2
declare -a list=( foo 'X Y' bar baz )
declare -a s
declare -A s1=( [foo]='FOO 1' [bar]='BAR 2' [baz]='BAZ beep; eject /dev/sr0' )
declare -A s2=( [foo]='oof oof' [bar]='rab rab' [baz]='zab zab' [X Y]='9')

# your code rewritten, we don't even need a counter
for j in "${list[@]}"
  do
    s+=( "${s1[$j]}" "${s2[$j]}" )
  done

# checking contents of s
printf '%s\n' "${s[@]}"

Hint: research "indexed arrays", "associative arrays" and their usage in bash.

Note the element with space (X Y) is nothing special to me; but it would break your code. I can also have BAZ beep; eject /dev/sr0 in my s1; I dare you to set s1baz='BAZ beep; eject /dev/sr0', have baz in your list array, execute your code and see what happens. This is what eval is capable of. Now imagine I put rm -rf ~/ instead of eject ….


The second example can be greatly simplified as:

sed -i "17i$sed17line" "$file"

You seem to have some bias towards eval + echo since you created a contraption that uses them twice when none is needed. Maybe it's because your variables not always expand to their values when you want this; if so, investigate the difference between single (') and double quotes (") in bash.

Kudos for having doubts and asking the question.

3
  • Thanks! I think you gave me too much information to process and I am gonna be careful in trying to get it all. I wouldnt say I have a bias, I'd say I have a lack of knowledge. When googling specifically for what I was trying to do all the answers I found in forums like this was to use eval and echo. It might just be the way I am thinking the steps of the code should be that lead me to those answers, but nonetheless they did, they worked and they made sense to me. Now I am trying to improve the code. – M.O. May 16 '18 at 23:39
  • The code has a very specific use, so I am not worried about the wrong usage of eval, I probably should be though. Would you say eval is portable? I am gonna do my best to understand the way you suggested though. – M.O. May 16 '18 at 23:41
  • Everything that naively uses user input and later on executes it (even when it is being part of a larger string) has the potential of being exploited to execute arbitrary code. This shouldn't matter too much as long as the script does not run with root permissions, however. Also, a common pattern is to use var=$(<prog>) which executes prog as a subprocess and assigns its output to variable. No eval is needed in this case. – T Nierath May 17 '18 at 3:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.