Hello please help me :)

Crusty · 12-06-2023, 12:27 PM

#!/bin/bash

echo enter a number
read n
count=0
while[$n=!0]
do
count=$((count+1))
n=$((n/10))
done
echo number of digits = $count

This is my code for counting the number of digit in a number but it doesn't coz the terminal says

enter a number
23
./countdigits.sh: line 6: while[23=!0]: command not found
./countdigits.sh: line 7: syntax error near unexpected token `do'
./countdigits.sh: line 7: `do'

I updated the software but the error is still there

Turbocapitalist · 12-06-2023, 12:39 PM

Welcome. You'll want to install the utility called shellcheck and use it often. There is also a web site of the same name which will also test shell scripts.

In the future, please wrap your scripts in [code] [/code] tags so that indentation is preserved.

With all the formalities aside, compare,

Code:

while[$n=!0]

versus

Code:

 while [ $n != 0 ]

See "man test"

Lnthink · 12-06-2023, 01:46 PM

#!/bin/bash

# Declare following two vars as integer
declare -i n
declare -i count

echo Enter a Number:
read n

# the way I compare to integers (not using the posix method you are trying)
# "-gt" for greater than
# "-gte" for greater than or equal
# "-ne" for not equal
# "-lt" for less then
# "-lte" for less than or equal
# Also - note the spacing around the brackets below... don't try to save any spaces, it won't work.

count=0
while [ $n -ne 0 ]
do
count=${count}+1
n=${n}/10
done

echo number of digits = $count

MadeInGermany · 12-06-2023, 03:33 PM

Quote:

Originally Posted by Lnthink

#!/bin/bash

# Declare following two vars as integer
declare -i n
declare -i count

echo Enter a Number:
read n

# the way I compare to integers (not using the posix method you are trying)
# "-gt" for greater than
# "-gte" for greater than or equal
# "-ne" for not equal
# "-lt" for less then
# "-lte" for less than or equal
# Also - note the spacing around the brackets below... don't try to save any spaces, it won't work.

count=0
while [ $n -ne 0 ]
do
count=${count}+1
n=${n}/10
done

echo number of digits = $count

The $(( )) was correct; don't make it worse!

Agreed, [ $n -ne 0 ] compares as numbers, and is straighter than [ $n != 0 ] that compares as strings (alphabetically).
The spaces are required because [ is actually a command that wants a ] as last argument. (This is compatible with the old archaic Bourne shell. But also the modern [[ ]], not yet in the Posix standard, needs the spaces.)

--

$(( )) enforces a numeric context and yields a value.
While (( )) enforces a numeric context and does not yield a value, and is not yet in the Posix standard.

Code:

#!/bin/bash

echo enter a number
read n
count=0
while ((n!=0))
do
  ((count++))
  ((n/=10))
done
echo "number of digits = $count"

teckk · 12-06-2023, 07:48 PM

Code:

read -p "Enter a number: " num
echo "Number of digits = "${#num}""

computersavvy · 12-06-2023, 07:50 PM

Just goes to show that with linux there are lots of ways to do the same thing.

GazL · 12-07-2023, 05:23 AM

If you're going to use user supplied input in numeric context inside a (( )), [[ ]] or $(( )) then you need to validate it first as numeric context is unsafe and can lead to shenanigans!

Code:

$ read n && [[ n -gt 0 ]] && echo yes
(PATH=0)
$ echo $PATH
0

... Yikes!

Code:

$ read n && (( n > 0 )) && echo yes
(PATH=0)
$ echo $PATH
0

... Yikes!

The same is true for a declare -i n ; read n construct, except that there's no way to validate the input as it's already too late:

Code:

$ declare -i n ; read n
(PATH=0)
$ echo $PATH
0

... Triple Yikes!

Code:

$ read n && [ "${n:-0}" -gt 0 ] && echo yes
(PATH=0)
bash: [: (PATH=0): integer expression expected
$ echo $PATH
/usr/local/bin:/usr/bin:/bin
$

... is a little safer: you still get a error, but at least it doesn't mess with your environment.

Moral of the story, always validate your inputs using a non-numeric context first.

bash:

Code:

read -p "Enter a number: " n || echo

if [[ "$n" =~ [^0-9] ]]; then
  echo "BAD INPUT!" >&2 ; exit 1
fi
...

If you're using a POSIX shell that doesn't have a regex match, then you can use a case statement:

Code:

case $n in
 "" | *[!0-9]* ) echo "BAD INPUT!" >&2 ; exit 1 ;;
esac

Now, if you're just writing scripts for yourself then you're not likely to try and exploit yourself, but it's wise to get into good habits.

MadeInGermany · 12-07-2023, 04:38 PM

This kind of code injection wasn't on my radar, thanks for pointing it out!

One comment: to not allow an empty input it should be

Code:

if [[ ! "$n" =~ ^[0-9]+$ ]]; then
  echo "BAD INPUT!" >&2 ; exit 1
fi

GazL · 12-08-2023, 04:18 AM

I decided to do a benchmark on the various ways I could come up with of validating the input using the output of seq 1 1000000.

Results

Surprisingly, I found that "case" was the fastest and most efficient:

Code:

$ cat /tmp/test-case 
#!/bin/bash

while read n
do
  case $n in
    *[^0-9]* ) exit 1 ;;
  esac
done < <( seq 1 1000000 )
$ time /tmp/test-case 

real    0m12.819s
user    0m8.324s
sys     0m6.593s
$

Next, but not that far behind, was a [[ ]] extglob pattern using 0-9

Code:

$ cat /tmp/test-pattern_0-9 
#!/bin/bash

while read n
do
  [[ "$n" != +([0-9]) ]] && exit 1
done < <(seq 1 1000000 )
$ time /tmp/test-pattern_0-9 

real    0m14.186s
user    0m9.615s
sys     0m6.474s
$

(note: I also tried using the == *[^0-9]* pattern similar to that used in the "case" here but the results were the same as for the extglob: suggesting that the performance loss over "case" is down to [[ ]] being used ).

Next up, and perhaps not unsurprisingly, an extglob using the [:digit:] abstraction:

Code:

$ cat /tmp/test-pattern_digit 
#!/bin/bash

while read n
do
  [[ "$n" != +([[:digit:]]) ]] && exit 1
done < <( seq 1 1000000 )
$ time /tmp/test-pattern_digit 

real    0m15.806s
user    0m10.285s
sys     0m7.083s

... not much in that one.

Now the difference start to get significant...

Next up: a [ ] test using -ge 0 and -le 0

Code:

$ cat /tmp/test-gele 
#!/bin/bash

while read n
do
  ! [ "$n" -ge 0 -o "$n" -le 0 ] 2>/dev/null && exit 1
done < <( seq 1 1000000 )
$ time /tmp/test-gele 

real    0m26.326s
user    0m18.732s
sys     0m9.724s
$

Last, and trailing far behind the rest: regex

Code:

$ cat /tmp/test-regex 
#!/bin/bash

while read n
do
  [[ ! "$n" =~ ^[0-9]+$ ]] && exit 1
done < <( seq 1 1000000 )
$ time /tmp/test-regex 

real    0m45.975s
user    0m35.188s
sys     0m11.226s
$

MadeInGermany · 12-08-2023, 04:47 AM

Quote:

even though '[' is an external command rather than a shell builtin

No it's a builtin command!

Code:

$ type [
[ is a shell builtin

The external command is /bin/[

Code:

time while read n
do
  ! /bin/[ "$n" -ge 0 -o "$n" -le 0 ] 2>/dev/null && exit 1
done < <( seq 1 1000000 )

pan64 · 12-08-2023, 04:53 AM

yes, regex is extremely slow in bash

Code:

[[ -n $n ]] || exit 1
[[ -z ${n//[0-9]/} ]] || exit 1

GazL · 12-08-2023, 04:59 AM

Quote:

Originally Posted by MadeInGermany

No it's a builtin command!
[

Yes, thankyou. I just realised that after posting and corrected my post. For some reason I'd gotten it into my head that [[ was builtin but [ wasn't. (probably because I remembered there's a [ in /usr/bin).

GazL · 12-08-2023, 05:04 AM

Quote:

Originally Posted by pan64

yes, regex is extremely slow in bash

Code:

[[ -n $n ]] || exit 1
[[ -z ${n//[0-9]/} ]] || exit 1

That one uses n in the unsafe numeric context we were trying to avoid by testing. edit: nevermind. Sorry, I misread what you had written (not going to post again until I've had another coffee!)

Second one is a good idea though. Using sub is slightly slower than the extglob solutions:

Code:

$ cat /tmp/test-sub 
#!/bin/bash

while read n
do
  [[ -n "${n//[0-9]}" ]] && exit 1
done < <(seq 1 1000000 )
$ time /tmp/test-sub

real    0m16.193s
user    0m12.101s
sys     0m6.483s
$

GazL · 12-08-2023, 05:09 AM

Mind you... if you're concerned about performance, you're not going to be using bash anyway.

pan64 · 12-08-2023, 05:10 AM

Quote:

Originally Posted by GazL

That one uses n in the unsafe numeric context we were trying to avoid by testing.

Second one is a good idea though. Using sub is slightly slower than the extglob solutions:

Code:

$ cat /tmp/test-sub 
#!/bin/bash

while read n
do
  [[ -n "${n//[0-9]}" ]] && exit 1
done < <(seq 1 1000000 )
$ time /tmp/test-sub

real    0m16.193s
user    0m12.101s
sys     0m6.483s
$

those should be used together, the second one alone does not work for empty strings, but you can skip the first step, if you know n is non-empty.
Otherwise you can do a numeric check too, something like this:

Code:

[[ $(( n + 2 )) -eq $(( n + 2 )) ]] || exit 1