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)) doneThe 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.
1 Answer
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
