Pipe with loops in shell script

Question

I have a text file with this information:

Game Copies_Sold(M) Release_date Genre   
God_of_War 19 20/04/2018 Action-Adventure 
Uncharted_4 16 10/05/2016 Action-adventure 
Spider-Man 13 07/09/2018 Action-adventure  
The_Witcher_3 10 18/05/2015 Action_role-playing

and I need to sum the numbers of the second column, so I wrote this script:

#!/bin/bash                                           
file=$1                                                   
s=0                                                      
tail -n +2 $file |cut -f 2 |                                
{ while read line                                          
do                                                      
s=$(( $s+$line ))                                        
done <$file }                                            
echo $s

but obviously I do something wrong. What's to be done here? Thank you!

Related: [Why is using a shell loop to process text considered bad practice?](https://unix.stackexchange.com/q/169716) — Stéphane Chazelas, Feb 01 '23 at 17:10
[Why is my variable local in one 'while read' loop, but not in another seemingly similar loop?](https://unix.stackexchange.com/q/9954/170373) — ilkkachu, Feb 01 '23 at 17:28

Stéphane Chazelas · Accepted Answer · 2023-02-01T17:32:35.840

Should be something like:

#! /bin/sh -
file=${1?}
awk 'NR >= 2 {sum += $2}; END {print sum+0}' < "$file"

Problems with your approach:

you do not check whether at least one argument is passed to your script (addressed here crudely with ${1?}).
Missing quotes around $file
Missing -- for tail to mark the end of options (nitpick: and same - in the shebang)
default separator for cut is TAB, while your input looks like it's space separated. For awk default separator is any amount of blanks (including space and tab).
You're piping to the loop, but also redirecting that loop's input from $file.
Missing ; or newline before }
Missing -r option to read
In bash, the right-most component of a pipeline also runs in a subshell (unless the lastpipe option is enabled), so changes to the $s variable after that pipeline returns would be lost.
but first you're using a shell loop to process text which in general is bad practice, inefficient and hard to get right.
here it's worse as you're using shell arithmetic on external input which introduces command injection vulnerabilities in bash.
there's nothing bash-specific in your script, so you can remove the bash dependency by changing the shebang to use the system's sh.

Several of those mistakes would have been spotted by shellcheck (also available as standalone software you can install on your system).

Here, it gives:

$ shellcheck myscript
 
Line 4:
tail -n +2 $file |cut -f 2 |                                
           ^-- SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: (apply this, apply all SC2086)
tail -n +2 "$file" |cut -f 2 |                                
 
Line 5:
{ while read line                                          
        ^-- SC2162 (info): read without -r will mangle backslashes.
 
Line 7:
s=$(( $s+$line ))                                        
^-- SC2030 (info): Modification of s is local (to subshell caused by pipeline).
      ^-- SC2004 (style): $/${} is unnecessary on arithmetic variables.
         ^-- SC2004 (style): $/${} is unnecessary on arithmetic variables.
 
Line 8:
done <$file }                                            
      ^-- SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: (apply this, apply all SC2086)
done <"$file" }                                            
 
Line 9:
echo $s
     ^-- SC2031 (info): s was modified in a subshell. That change might be lost.

In most `awk` versions you may not need `'NR > 2` because `awk` will interpret the string as zero — Romeo Ninov, Feb 01 '23 at 17:17
@RomeoNinov, true but only as long as the header for that second column doesn't start with something that looks like a number. For instance, if that were `Infamous_Copies_Sold(M)`, you'd get `inf` or `+inf` on output because that string starts with `Inf` which is taken as the infinity floating point number. Best it to skip that first line if it's not meant to be counted. — Stéphane Chazelas, Feb 01 '23 at 17:55

Pipe with loops in shell script

1 Answers1