[SOLVED] Linux bash scripting question

nickygencs17 · 04-15-2014, 11:36 AM

hey I'm having a little trouble here hoping someone can help me

write a script that takes multiple directory names as an argument and writes to standard output the maximum of all the lengths of all the filenames in those directories. if the functions argument is not a directory or lacking permissions to access the directory write an error message to standard error. however keep processing the rest if any valid arguments remain at the end exit with a non zero status. the length only takes into account the base name of a file

what i have so far

#!/bin/bash
#nicholas Genco
count=0
longfile=" "
if [ "$@" -eq 0 ]
then
echo "not enough arguments exit 567"
else

for i; do
if [ -d "$i" -a -r "$i" ]
then
s=[ echo "$i" | wc -c ]

if[ "$s" < "$count" ]
then
count=$s
longfile=$i
fi
else
echo "not a directory or not acessible"> /dev/sterr
fi
done

pan64 · 04-15-2014, 12:08 PM

please use [code]here comes your code[/code] to keep original formatting of your script
please insert set -xv as the third line into that script and check the messages....

there are a few problems, like:

Code:

if[ "$s" < "$count" ]
#better:
if [ "$s" < "$count" ]

Code:

s=[ echo "$i" | wc -c ]
#this is definitely not ok, what would you like to do here?

nickygencs17 · 04-15-2014, 12:22 PM

i wanna count the number of characters in the filename

Code:

#!/bin/bash
#nicholas Genco
count=0
longfile=" "
if [ "$@" -eq 0 ]
then
        echo "not enough arguments exit 567"
else

 for i; do
        if [ -d  "$i" -a -r "$i" ]
        then
                s=[ echo "$i" | wc -c ]
                        
        Connection closed by foreign host." ]
Nicholass-MacBook-Pro:~ nick_genco$ 
                        count=$s
                        longfile=$i
                        fi
        else
        echo "not a directory or not acessible"> /dev/sterr
        fi
done

pan64 · 04-15-2014, 12:24 PM

s=$( echo "$i" | wc -c )
would work better
also do not forget about set -xv

nickygencs17 · 04-15-2014, 12:38 PM

thank you i appreciate the help although still getting an error on line 15 for the then

nickygencs17 · 04-15-2014, 12:40 PM

Code:

 1 #!/bin/bash
  2 count=0
  3 set -xv ongfile=" "
  4 
  5 if [ "$@" -eq 0 ]
  6 then
  7         echo "not enough arguments exit 567"
  8 else
  9 
 10  for i; do
 11         if [ -d  "$i" -a -r "$i" ]
 12         then
 13                 s=$(echo "$i" | wc -c )
 14                         if[ "$s" < "$count" ]
 15                         then
 16                         count=$s
 17                         longfile=$i
 18                         fi
 19         else
 20         echo "not a directory or not acessible"> /dev/sterr
 21         fi
 22 done
 23 fi

pan64 · 04-15-2014, 01:58 PM

yes, as I told you there is an error in line 14. please see my first post

grail · 04-15-2014, 09:39 PM

I would probably use bash's builtin to get the length:

Code:

s=${#i}

And for the testing of numbers I find the round brackets and the use of arithmetic symbols on numbers as opposed to string in your example:

Code:

if (( s < count ))

I also prefer the use of && / || over -a / -o as they stand out better, especially when using the test switches, ie -d, -r, etc

I am not sure I understand your first test either?

Code:

if [ "$@" -eq 0 ]

Why would $@ be equal to zero?? I understand that the user may have not used any command line arguments, but this would make $@ empty or null, which is not equivalent to zero

Lastly, you seem to store the count and longfile name but then do nothing with them? (ie they are not displayed to the user on screen)

michaelk · 04-15-2014, 10:35 PM

To elaborate on grail's post. I assume you actually want to check for the number of parameters. Did your teacher cover $#, $* and $@?

The following link should provide somewhat of a hint:
http://www.ibm.com/developerworks/op...ers/index.html

chrism01 · 04-16-2014, 06:55 AM

A couple more tips:
1. put 'set -xv' on its own line
2. http://tldp.org/LDP/abs/html/testcon...ml#DBLBRACKETS

nickygencs17 · 04-24-2014, 10:56 AM

hey thanks for the help guys here the finished product

Code:

#!/bin/bash
#Nicholas Genco
count=0; #setting count to 0
longfile=" "; #setting longfile to empty string

if [ "$#" = "0" ]; #if no arguments echo not enough arguments
then
        echo "Not Enough Arguments"  
        exit 567
else
for i; do
        if [ -d "$i" ]
        then
                if [ -a "$i" ]
                then
                        if [ -r "$i" ]
                        then
                                s=$(ls -l "$i" | grep ^- |tr -s "[:space:]"|cut -d" " -f9)
                                for j in $i/*; do
                                        s=$(ls -l "$i" | grep ^- | tr -s "[:space:]" | cut -d" " -f9 ); #SETTING MAX CHARACTER FILE INT TO S 
                                        bn=$(basename "${j}")

                                        if [ "${#bn}"  -gt "$count" ]; #TESTING TO IF S IS GREATER THEN COUNT 
                                        then
                                        count=${#bn}; #SETTING S TO COUNT 
                                        longfile=$j;    #SETTING FILENAME TO LONGFILE
                                        fi
                                done
                        else
                        echo "Directory Not Acessible" >/dev/stderr; #DIRECTORY IS NOT ACCESSIBLE TO STDERROR
                        exit 123
                        fi
                fi
        else
        echo "Directory Does Not Exist" >/dev/stderr; #directory does not exist to stderr
        exit 233
        fi

done
fi
echo "$count"; #ECHOING COUNT AND FILENAME 
echo "$longfile"
exit 0

grail · 04-24-2014, 01:14 PM

Here are some pointers. You are of course free to ignore any you do not like

1. You supply error messages, which would generally lead me to believe others may use your code. Why then is there no instruction on how to call your code correctly? You advise the mistake but not how
to correct it??

2. You set the variable longfile, however your comment advises it is to an empty string. This is not technically correct as it now contains a space, ie. not empty. Curiously, should we bypass all
the previously caught errors you will echo this single space to the screen. Maybe you should be testing if longfile is still a single space or count is greater than zero before delivering the result.

3. Your first error message says "Not Enough Arguments", so how many is enough?? (see point 1)

4. Inside the for loop your first test is to whether or not a directory was passed and on error a message goes to stderr, but the you exit the script. So if any are not directories then none of the other
items in the list are to be processed? (again point 1 & 3 may mean only one directory is to be passed)
Also, how does this user know which one they typed in wrong??

5. I do not understand the point of the next test, ie -a "$i"? It has no else nor any associated error, so I am wondering if it is required at all??

6. See point 4 about next test advising the directory is not accessible but also no advice on which one

7. Do not parse ls for reason stated here. As an example, if I take your snippet below and use it on my working directory I am returned a blank line for all files as they are in the
the 8th position and not the 9th which is returned by your cut command:

Code:

ls -l | grep ^- |tr -s "[:space:]"|cut -d" " -f9

Also, if the files have spaces in them you will get some very weird results.

8. You set the value for the variable 's' both before the inner for loop and immediately inside. This would seem to serve no purpose

9. Your comment on setting variable 's' inside the loop says, #SETTING MAX CHARACTER FILE INT TO S, I am not sure I understand? Are you saying that the last file in every directory will always
have the maximum number of characters? (again another reason not to parse ls)

10. After all the different points where you set the 's' variable, it is never used???

11. You test the length against your count variable, but you quote them as if they are strings and then use '-gt' for testing numbers. As advised previously, if you use round brackets for arithmetic,
it is clearer on what you are testing:

Code:

if (( ${#bn} > count ))

12. You set the count and longfile values but at the end you advise that you are displaying the count and the file name, however, the count is of only the file name whilst you are setting the
longfile value to the entire path where the file was found, ie. this will not be equal to the count shown

Well I hope some of the above is useful to you

nickygencs17 · 04-24-2014, 04:47 PM

SORRY still new at this...Thanks for everyone who helped me especially grail ***HERE IS THE REFINISHED PRODUCT***

Code:

#!/bin/bash
#Nicholas Genco
count=0; #setting count to 0
longfile=""; #setting longfile to empty string

if [ "$#" = "0" ]; #if no arguments echo not enough arguments and exit
then
        echo "Not Enough Arguments: Please Enter 1 or More"  
        exit 567
else
for i; do
        if [ -d "$i" ]; #Testing arguments to see if directory exists 
        then
                if [ -r "$i" ]; #checking if the directory is readable 
                        then
                        for j in $i/*; do
                                bn=$(basename "${j}"); #setting bn to the string following the final '/'.
                                        if (( ${#bn} > $count )); #Testing if the number of characters in bn are greater then the count's int 
                                        then
                                                count=${#bn}; #if bn is greater then count bn= new count 
                                                longfile=$j;    #SETTING j = file path
                                        fi
                        done
                else
                echo "Error: ""$i" "= Directory Not Accessible" >/dev/stderr; #DIRECTORY IS NOT ACCESSIBLE TO STDERROR
                fi
        else
        echo "Error: ""$i" "= Directory Does Not Exist" >/dev/stderr; #directory does not exist to stderr

        fi

done
fi
if (($count > 0))
then
        bn1=$(basename "${longfile}"); #using basename to set bn1 to the string following the final '/'.
        echo "Longest File: ""$bn1"
        echo "Number of Characters: ""$count"; #echoing the longest file and the name of the longest file 
        exit 0
else
echo "Error: Longest file = '0'"
fi

grail · 04-25-2014, 01:48 AM

Nice work

I only have 2 points / questions:

1. Why have you separated your error messages into several quoted strings?

Code:

# current
echo "Error: ""$i" "= Directory Not Accessible" >/dev/stderr;

# alternative
echo "Error: $i= Directory Not Accessible" >/dev/stderr;

As you can see, only a single set of quotes really required. Also, the semi-colon at the end is not required, although in the current positions will cause no issues

2. Instead of setting the longfile variable to what is stored in 'j', why not use what is stored in 'bn' as this is the string you are using for the length?