unable to read a file in while loop

Question

I've the below code:

#!/bin/bash

. ~/.bash_profile

dt=$(date +"%Y.%m.%d")

if [[ "$#" -eq '0' ]] || [[ "$#" -eq '1' ]]
then
echo -e "Not correct usage"
exit 1
fi

tar zvxf $SHARE/*.gz -C $SHARE/landing/

sleep 3

ops () {
//
some sets of command
//
}

while read -r line
do
if [[ "$1" == "A" ]] || [[ "$1" == "B" ]] || [[ "$1" == "C" ]] && [[ "$2" =~ "$line" ]] || [[ "$2" == "ALL" ]]
then
ops
fi
done < $SHARE/landing/dir/ApprovedList."$1"

When i run the script, i can see that it's able to untar the .gz file and place it into the folder :$SHARE/landing/

But i guess it's not able to read the file and perform the operations inside the ops function, below is the last line of output i'm getting after running the script in interactive mode:

+ read -r line

Any help is most welcome!!!

May I recommend using shellcheck (https://www.shellcheck.net/) on the script first. — , May 20 '19 at 12:21
does the `ops` function include programs that consume stdin? — glenn jackman, May 20 '19 at 12:22
@glennjackman: basically ops function contains some cp,mv commands and uses the variable `line`. — A.K, May 20 '19 at 12:31
Look into shellcheck.net, your `=~` may not do what you expect. — Michael Jaros, May 20 '19 at 12:44
Maybe a 'mv' inside the ops function is trying to overwrite an existing file and is asking you to answer 'yes' or 'no'. If you always want to overwrite, use `cp --force ...` and `mv --force ...`. — , May 20 '19 at 12:51
@MichaelJaros: I checked in shellcheck.net, doesn't seems to be an issue. — A.K, May 20 '19 at 13:14
I think you should let us in on what exactly the function 'ops' does. This way it's a blackbox, and answering your question remains a guessing game. — , May 20 '19 at 13:52
That multiple condition `if` with all the embedded `||` and `&&` might work, but looks too fragile to me. From the [BashFAQ}(https://mywiki.wooledge.org/BashGuide/TestsAndConditionals#Control_Operators_.28.26.26_and_.7C.7C.29): "It's best not to get overzealous when dealing with conditional operators. They can make your script hard to understand, especially for a person that's assigned to maintain it and didn't write it themselves." Don't try to do everything in one command. Maybe the `&&` represents an embedded pair of `case` statements. :) Also, some error checking/handling seems in order. — Paul Hodges, May 20 '19 at 14:25
You have not shown us the contents of the file, or the command line parameters. There's no way we can help you. Voting to close as unclear. — glenn jackman, May 20 '19 at 14:47
How do you think `ops` is using the line? You didn't pass it as an argument. Is it reading it from stdin? If so, see @glennjackman's comment. — William Pursell, May 20 '19 at 14:51
The `while read` loop won't process the last line in the input file if it's not terminated (a fairly common situation). See [Read a file line by line assigning the value to a variable](https://stackoverflow.com/q/10929453/4154375). — pjh, May 20 '19 at 15:04
If this were a [mcve], it could just be three lines. One `while read -r line; do` line, one `echo "$line"`, and a `done`... plus whatever the *shortest possible* amount of extra complexity needed to cause the actual bug is. Here, we have something that's basically certain to be completely unrelated to the while loop. — Charles Duffy, May 20 '19 at 15:42

Paul Hodges · Accepted Answer · 2019-05-20T14:57:26.503

2

Please show the input format, the argument format, and some explanation of those. In the meantime, tweaked for readability and error handling - edit to taste. YMMV.

#!/bin/bash

. ~/.bash_profile

ops () {  # do some stuff
  : some sets of commands ...
}

die() {
  printf "%s\n\n Use: $0 x y z ...\n\n" "$1" >&2
  kill -term $$ # exit in function behaves like return
}

# dt=$(date +"%Y.%m.%d") # unused

case "$#" in
0|1) die "Insufficient arguments" ;;
esac

tar zvxf "$SHARE"/*.gz -C "$SHARE/landing/" || die "tar failed"

sleep 3
infile="$SHARE/landing/dir/ApprovedList.$1"
[[ -e "$infile" ]] || die "$infile does not exist"
[[ -r "$infile" ]] || die "$infile is nor readable by $0 as $LOGNAME"
while read -r line
do case "$1" in
   A|B|C) case "$2" in
          *"$line"*|ALL) ops ;;
                      *) : data does not match, skipping ;;
          esac ;;
       *) die "Invalid arg1 '$1'";;
   esac
done < "$infile"

Notes:

If you require exactly 2 arguments, then let's check exactly that:

die() {
  printf "%s\n\n Use: $0 x y\n\n" "$1" >&2
  kill -term $$ # exit in function behaves like return
}

and

case "$#" in
2) ;;
*) die "Incorrect # of arguments" ;;
esac

Also, better than the kill - add a trap at the top:

trap 'echo >&2 "ERROR in $0($BASH_SOURCE) at line $LINENO: [$BASH_COMMAND]"; exit $LINENO;' ERR

and use a literal error return in the function.

die() {
  printf "%s\n\n Use: $0 x y\n\n" "$1" >&2
  return 1
}

edited May 20 '19 at 14:57

answered May 20 '19 at 14:48

Paul Hodges

13,382
1
17
36

1

`exit` in a function behaves like return only if the function is in a subshell. – William Pursell May 20 '19 at 14:53
That's an excellent point - but unless this whole script is being `source`d, it would be, yes? Still, thanks for the clarification. – Paul Hodges May 20 '19 at 14:55
1

I think you want `case "$2" in "*$line*"|ALL) ops ;;` to mimic the `=~` operator. – William Pursell May 20 '19 at 14:56
Although that implies that the OP meant to write `[[ $line =~ $2 ]]`, which seems the more natural thing to write. – William Pursell May 20 '19 at 15:01
But he needs to be able to skip the line matching if `$2` was `ALL`, yes? – Paul Hodges May 20 '19 at 15:05
I think it could be written `[[ $2 == ALL ]] || [[ $line =~ $2 ]]`, but your case statement is much better. – William Pursell May 20 '19 at 15:24
Err, surely that should be `in *"$line"*|ALL) ops ;;`, with the `*`s unquoted? – Charles Duffy May 20 '19 at 15:37
Yeah, I fixed that when I added it in. – Paul Hodges May 20 '19 at 16:51

Walter A · Answer 2 · 2019-05-20T15:48:08.273

0

The check (except for [[ "$2" =~ "$line" ]])

if [[ "$1" == "A" ]] || [[ "$1" == "B" ]] || [[ "$1" == "C" ]] && [[ "$2" =~ "$line" ]] || [[ "$2" == "ALL" ]]

doesn't change inside the while-loop. So better check once before entering the while-loop. So now you want to perform ops for each line of $SHARE/landing/dir/ApprovedList."$1".

You can use

xargs -a "$SHARE/landing/dir/ApprovedList.$1" -L 1 ops

EDIT: When you have a simple check to perform for each line, you can move this check into the ops function.
First save $2 before entering the function:

to_be_checked="$2"
...
ops() {
   [[ "$0" =~ "${to_be_checked}" ]] && return

edited May 20 '19 at 15:48

answered May 20 '19 at 15:32

Walter A

19,067
2
23
43

...well, *exactly one* condition changes, `[[ "$2" =~ "$line" ]]`. Though that is very, very unlikely to be what the OP really wants (do they really intend to check if the line is an *exact substring* of the 2nd argument? The other way around, or the other way around *and* doing a regex check rather than a substring check, seems more likely). – Charles Duffy May 20 '19 at 15:36
@CharlesDuffy I was partially blind. First I wanted to suggest to use `case "$1" in` but noticed a `$2`. Then I stopped processing that line. – Walter A May 20 '19 at 15:50

unable to read a file in while loop

2 Answers2

Notes: