0

First post on StackOverflow, first time using Bash.

I'm currently trying to construct a function to read a file, then give me out put depending on the status:

  • If it's a file, then print the name, size and type
  • If it's a symbolic link, then print the name and it's target
  • If it's a directory, then recursively call the function on each item.
  • Otherwise return 'unknown'

I know the code is rough so far, but all I really need help with is the recursive looping part. How do I loop through the contents of a directory using a While Loop?

Here's my code so far:

#!/bin/bash

function fileChecker f  {

    if [-e $1 ]; then
        if [ -f $1]; then 
            SIZE = 'du -k $1 | cut -f1'
            CONTENT = 'file $1 | cut -d':' -f2'
            echo $1 +" " +  $SIZE + " " + $CONTENT
    elif[ -h $1 ]; then
        LINK = get info from $1
        echo $1 + " " + "symbolic link " + $LINK
    elif [ -d $1 ]; then 
        while  [ ???? ]; do  
            fileChecker n
        do
    else
        echo $1 + "unknown"
    fi
fi 
}

Thanks in advance!

  • 1
    You've got a bunch of bugs here (unrelated to the looping) that http://shellcheck.net/ will catch. Because this is several bugs, not one, it's not a great scope for a single question. – Charles Duffy Oct 09 '17 at 19:38
  • Could you please link to the question you mention? I'd love to take a look at it for notes... – Andrew Messina Oct 10 '17 at 19:02
  • 1
    randomir has actually done a really good job of calling out specific issues (also, if your run your code through http://shellcheck.net/, see the wiki links in its results). That said, I did link to four questions regarding specific bugs present in the originally presented code -- see the list above, under the "This question already has an answer here:" header at the top. – Charles Duffy Oct 10 '17 at 21:00

1 Answers1

3

There's nothing stopping you to call your function recursively, like this:

fileChecker() {
    # ...
    for f in "$1"/*; do
        fileChecker "$f"
    done
}

Here's your script with a few fixes (*), behaving as described:

#!/bin/bash
fileChecker() {
    if [ -f "$1" ]; then 
        size=$(du -k "$1" | cut -f1)
        content=$(file "$1" | cut -d':' -f2)
        echo "$1 $size $content"
    elif [ -h "$1" ]; then
        link=$(readlink -f "$1")
        echo "$1 symbolic link $link"
    elif [ -d "$1" ]; then 
        for f in "$1"/*; do
            [ -e "$f" ] || [ -h "$f" ] && fileChecker "$f"
        done
    else
        echo "$1 unknown"
    fi
}

(*) Fixes:

  • correct spacing before and after [ command
  • correct quoting of variables ("$1")
  • omitted redundant checking of file existence, since -f already checks if file exists (and is regular)
  • using command substitution $(cmd ...) instead of backticks (simpler and more readable)
  • string concat fixed in echo
  • using readlink -f for symlink target
  • POSIX-compliance (function definition)

Just note that for f in "$1"/* will not loop over filenames starting with a dot ("hidden files"). Also note the [ -e "$f" ] || [ -h "$f" ] check before a call to fileChecker to prevent calling it on "$1"/* in case the glob does not expand (when there are no files in dir "$1").

randomir
  • 15,849
  • 1
  • 31
  • 46
  • Hmm. Might want to try to handle empty directories a bit better -- you'll get `unknown` printed for the nonexistent file `*` therein returned by the glob expression. Other than that, LGTM. – Charles Duffy Oct 09 '17 at 19:43
  • @CharlesDuffy thanks, have a look now. Any ideas for better handling (this doesn't seem too elegant)? – randomir Oct 09 '17 at 19:55
  • `[ -e "$f" ] || [ -L "$f" ]`, if we want to recurse through symlinks whose targets don't exist. You could potentially test only the first result of `"$1"/*` if you wanted to (`set -- "$1"/*; [ -e "$1" ] || [ -L "$1" ] || return; for arg; do fileChecker "$arg"; done`), but I'm not sure if that's worth it. – Charles Duffy Oct 09 '17 at 19:59
  • Hmm, yes. But I don't think recursing over non-existing symlink is what OP wants. Other than that, look good? Thanks, Charles. – randomir Oct 09 '17 at 20:02
  • Yup, I think so. This has had my upvote for a while (since your first edit switching over to POSIX-compliant function declaration syntax). – Charles Duffy Oct 09 '17 at 20:03
  • (That said, I'm inclined to think that recursing into a symlink with a nonexistent target is the right thing -- otherwise you don't print the `symbolic link` line for it). – Charles Duffy Oct 09 '17 at 20:04
  • That makes sense, I'll make it so. :) – randomir Oct 09 '17 at 20:06
  • Btw, I really appreciate all your tips, thanks again, Charles! – randomir Oct 09 '17 at 20:16
  • BTW, `[ -e "$f" -o -L "$f" ]` is syntax that's marked obsolescent [in the POSIX `test` standard](http://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html); search for the string `[OB XSI]`. Granted, the *reason* they're trying to remove it relates to ambiguities that can't come up with the restricted set of possible values in the usage here (even if you have `f='('`, things only get *really* ambiguous when you have another variable containing the exact string `)` in use in your data later). – Charles Duffy Oct 09 '17 at 21:44
  • Alright, no more `-o`. :) – randomir Oct 10 '17 at 00:19
  • Thank you both so much for your help! It looks like a lot of my coding is being informed by an obsolete site. Can either of you recommend a site that has more modern 'best practices' ? – Andrew Messina Oct 10 '17 at 19:06
  • @AndrewMessina, for learning Bash, see http://mywiki.wooledge.org/, and for a quick reference, see http://wiki.bash-hackers.org/. – randomir Oct 10 '17 at 19:07
  • Also, is it not possible (ie... too complicated) to use a while loop? I've seen the 'for f in $VARIABLE' notation before, but I thought it could be done a different way. – Andrew Messina Oct 10 '17 at 19:21
  • @randomir thanks for those sites! I'll bookmark them now. ^_^ – Andrew Messina Oct 10 '17 at 19:22
  • @AndrewMessina, it's possible, but it's simpler with `for`. – randomir Oct 10 '17 at 19:23